AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
941 stars 335 forks source link

windows: path-separator issues using {root} with package attribute tests={} #508

Open willjp opened 6 years ago

willjp commented 6 years ago

Hey guys,

On windows, where os.normpath() prefers back-slashes to forward slashes I ran into an issue:

# package.py

tests = {
    'unit':{
        'command': 'pytest -o "testpaths={root}/tests"',
        'requires': ['pytest'],
    }
}
Running test command:  pytest -o "testpaths=C:\Users\vagrant\Documents\packages\testpkg\0.0.1/tests"
ERROR: file not found: C:Usersvagrantpackagestestpkg0.0.1/tests

I tried sneaking around it and using `'command': 'pytest -o "testpaths={}/tests"'.format( '{root}'.replace('\','/') )`` , but I'm guessing that the python code is run before the preprocessor.

I was wondering if any of you had any ideas to work around the unescaped backslashes. Thanks again for all of the help,

Will

instinct-vfx commented 6 years ago

Well this should be fixed I assume, but in the meantime you can also access things like {root} as python objects using this.name.

so for example this.root is the python equivalent to the {root} part in strings.

Cheers, Thorsten

willjp commented 6 years ago

Thanks again Thorsten!

Unfortunately that won't work, this only seems to be in scope within commands() :

rez.exceptions.ResourceError: Problem loading C:\Users\vagrant\Documents\examples\testpkg\package.py: name 'this' is not defined
instinct-vfx commented 6 years ago

Ah i see, but you might be able to work around that by implementing tests as an early binding function. See https://github.com/nerdvegas/rez/wiki/Package-Definition-Guide#early-binding-functions

willjp commented 6 years ago

Thanks very much, but that won't work as a workaround either. this is accessible, along with user-defined package-attributes, but this.root isn't bound yet:

@early()
def tests():
    root = str(this.root).replace('\\','/')
    return {
        'unit':{
            'command': 'pytest -o "testpaths={}/tests"'.format(root),
            'requires': ['pytest'],
        }
    }
AttributeError: No such package attribute 'root'
nerdvegas commented 6 years ago

Hey Will,

I think the trick here is to not attempt to expand {root} at package definition time (which doesn't work because there isn't a variant yet because the package isn't installed yet!). You want to do something like:

@early()
def tests():
    import os.path
    return {

    'unit':{
        'command': 'pytest -o "testpaths={root}%stests"' % os.path.sep,
        'requires': ['pytest'],
    }
}

It seems like I should make available builtin expansion for {sep} and {pathsep} so that this can be solved more trivially (is more trivially a thing?).

A

On Wed, Apr 18, 2018 at 9:35 PM, Will Pittman notifications@github.com wrote:

Thanks very much, but that won't work as a workaround either. this is accessible, along with user-defined package-attributes, but this.root isn't bound yet:

@early()def tests(): root = str(this.root).replace('\','/') return { 'unit':{ 'command': 'pytest -o "testpaths={}/tests"'.format(root), 'requires': ['pytest'], } }

AttributeError: No such package attribute 'root'

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/508#issuecomment-382356730, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSjDqqyDQ4b6tWUB8O5VSu0-7pjisks5tpyUQgaJpZM4TZMCF .

willjp commented 6 years ago

Thank you very much! That still does not solve the problem for me though,

It returns me to the original problem where the \ characters within {root} are not being escaped somewhere before they hit the commandline.

Running test command: pytest -o "testpaths=C:\Users\vagrant\packages\testpkg\0.0.1/tests"
======================= test session starts ===================
platform win32 -- Python 2.7.11, pytest-3.5.0, py-1.5.3, pluggy-0.6.0
rootdir: C:\Users\vagrant\packages\test-pkg, inifile:
=================== no tests ran in 0.01 seconds ================
ERROR: file not found: C:Usersvagrantpackagestestpkg0.0.1/tests

In the meantime, I can still run tests locally - rez-env sets up the environment/PYTHONPATH, and if I am in the directory with the tests, the tests will run.

nerdvegas commented 6 years ago

Ah, indeed. I should just make this expand to forward slashes when on Windows.

I see that 'tests' isn't in the list of late-binding-supporting attributes which is a shame, as you could have done that. I need to revisit this though, it may make sense to switch to a blacklist (ie only certain attribs

So is the problem that os.normpath uses backslashes even on Windows?

Thx A

On Thu, Apr 26, 2018 at 10:24 PM, Will Pittman notifications@github.com wrote:

Thank you very much! That still does not solve the problem for me though,

It returns me to the original problem where the \ characters within {root} are not being escaped somewhere before they hit the commandline.

Running test command: pytest -o "testpaths=C:\Users\vagrant\packages\testpkg\0.0.1/tests" ======================= test session starts =================== platform win32 -- Python 2.7.11, pytest-3.5.0, py-1.5.3, pluggy-0.6.0 rootdir: C:\Users\vagrant\packages\test-pkg, inifile: =================== no tests ran in 0.01 seconds ================ ERROR: file not found: C:Usersvagrantpackagestestpkg0.0.1/tests

In the meantime, I can still run tests locally - rez-env sets up the environment/PYTHONPATH, and if I am in the directory with the tests, the tests will run.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/508#issuecomment-384621710, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSopLYbTad19Gp1xf5eoytvBTVItXks5tsbx6gaJpZM4TZMCF .

willjp commented 6 years ago

That is my guess, but I haven't actually dove into the code to see - so it's entirely an assumption. I'm guessing that os.path.normpath is introducing the backslashes as path-separators, and something in between it and the call to subprocess is treating them as escape-characters.

Either the possibility of late-binding, or using forward-slashes would fix it for me - I don't want to break anyone else's workflow though, if they are expecting backslashes.

I'm in a crunch right now, but when I have a little time, I'll take a look at the code to see if I can figure out the general area the problem is ocurring.

Thank you again, for looking into this and helping me troubleshoot,