Remove trailing os.sep to avoid false negatives

[andresdelfino]

When we look the dirs in PATH, we don't remove the trailing os.sep, and since we are using a set to do the checking, we get false negatives.

Example:

[cjerdonek]

There is no test for this?

[andresdelfino]

No, there isn't, sorry. Please let me know if you want me to write it.

[cjerdonek]

As a simpler step, rather than only trying to test message_about_scripts_not_on_PATH() (which is a function with a lot of complexity), I would create and test a simpler and more easily testable helper function (e.g. make_not_warn_dirs() or however it should best be called).

[pradyunsg]

Please let me know if you want me to write it.

Yes, please do. :)

[andresdelfino]

I have created the test on #5325

[florisla]

Note: if it is at all possible to install scripts in the root folder of a file system, this might break.
Then the trailing separator is stripped from / on Linux or C:\\ on Windows, where it is also the root separator.

Using os.path.normpath() instead on the entries of PATH works equally well in my case, but I'm not sure if that introduces other undesired effects.

[andresdelfino]

The documentation does mention an issue with symbolic paths in Unix. Perhaps we should only remove the trailing os.sep in Windows and if not os.path.normpath(i).endswith(os.sep) ? This won't remove it in "C:\".

[florisla]

I like that!

This seems to work:
florisla@b756eed

I'm not doing the endswith(os.sep) since that check is already built into rstrip(os.sep).

[florisla]

I can add unit tests for this if you want. I would need some pointers on how to get the unit tests rolling though (currently getting ImportError: attempted relative import with no known parent package).

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.