margru / pybuilder-smart-copy-resources

PyBuilder plugin for copying additional resources
MIT License
0 stars 2 forks source link

Incorrect default properties smart_copy_resources_basedir #5

Closed AlexeySanko closed 7 years ago

AlexeySanko commented 7 years ago

If I use default value for smart_copy_resources_basedir plugin cannot find source directory:

[INFO]  Building distribution in /media/sf_Ubuntu-01/r1-ds-py-lib/target/dist/r1dspylib-1.0.0.dev
[DEBUG] Copying module/ package r1dslib
[INFO]  Copying scripts to /media/sf_Ubuntu-01/r1-ds-py-lib/target/dist/r1dspylib-1.0.0.dev/scripts
[DEBUG] Executing subtask from pybuilder_smart_copy_resources
[INFO]  Copying additional resource files
[WARN]  No files found to copy in smart_copy_resources for pattern: rpm/r1dspylib_settings.yaml

Pybuilder command project.expand_path add basic project path. In this case project.set_property_if_unset("smart_copy_resources_basedir", project.basedir) and copy_source_dir = project.expand_path(project.get_property("smart_copy_resources_basedir")) for path like /some/path/to/project produce path like /some/path/to/project/some/path/to/project.

margru commented 7 years ago

Hmm, this is strange. I supposed that the project.basedir attribute contains an absolute path (although the documentation doesn't provide much help here) and then joining absolute paths should keep only the last one in the final path. From your example /some/path/to/project, the project.basedir is absolute and thus joining it via os.path.join() (inside expand_path()) should keep it only once.

AlexeySanko commented 7 years ago

I just added prints

    # Get the properties
    copy_source_dir = project.expand_path(project.get_property("smart_copy_resources_basedir"))
    resources_to_copy = project.get_property("smart_copy_resources")
    print "################"
    print copy_source_dir
    print resources_to_copy
    print "################"

and checked result

[INFO]  Copying additional resource files
################
/media/sf_Ubuntu-01/myproject/media/sf_Ubuntu-01/myproject
{'??????????': '?????'}
################
[WARN]  No files found to copy in smart_copy_resources for pattern: rpm/r1dspylib_settings.yaml

As You can see path /media/sf_Ubuntu-01/myproject was doubled.

margru commented 7 years ago

Well, I believe you but I wonder how is that possible. Could you please add printouts for project.basedir and project.get_property("smart_copy_resources_basedir")? Otherwise, I am starting to think that expand_path() doesn't work the way it's described at http://pybuilder.github.io/documentation/api/core.m.html#pybuilder.core.Project.expand_path.

Thanks

AlexeySanko commented 7 years ago
    print "###########"
    print project.basedir
    print project.get_property("smart_copy_resources_basedir")
    print project.expand_path(project.get_property("smart_copy_resources_basedir"))
    print "###########"
###########
/media/sf_Ubuntu-01/myproject
/media/sf_Ubuntu-01/myproject
/media/sf_Ubuntu-01/myproject/media/sf_Ubuntu-01/myproject
###########

Unfortunately, pybuilder documentation is not ideal and we need to do a lot there.

margru commented 7 years ago

Hmm, now I see the problem. expand_path() is doing the following: self.expand(format_string).split(PATH_SEPARATOR) which will make the absolute path in format_string relative (it removes the leading path separator).

I am not sure if this is a correct behaviour of the expand_path() method. Why is it doing the split() part? Shouldn't it just keep the input format_string as is? It's hard to say as there is no documentation for this method which would tell us what it is supposed to do exactly.

Anyway, I will have to accept your workaround although I think it's not perfect (as the default directory is now not defined anyhow, just assuming that the project base directory is always prepended).

margru commented 7 years ago

Merged in #7

AlexeySanko commented 7 years ago

Hm... I think that we can found ideal workaround into original copying plugin.

AlexeySanko commented 7 years ago

Unfortunately, as I can see expand_path() works only with relative paths. :(