pyang
pyang copied to clipboard
Fix functionality on Windows
We've started using pyang
in a test suite of our project for linting the YANG files. Our test suite simply tried to execute pyang something something
, which failed on Windows. Upon investigating, it turned out that Wheel-based installation won't include the .bat
files, and upon investigating this further, it seems that the code tried to wrap Bash shell script with a Python shebang. Hence:
scripts: port to entry_points.console_scripts
The current approach is not recoomended, and it has not worked on Windows since the distribution switched to Python wheels. With wheels, no code from setup.py
runs at the installation time, which means that platform detection logic which tried to prepare BAT files on Windows was non-effective. The wheels are marked using the any
tag which means "pure Python code only", which is a correct marking, but then the actual content of wheels would differ depending on whether they were built on Windows vs. Unix.
The modern alternative is to let setuptools
create these scripts automatically. That thing apparently puts, e.g., pyang
into $PATH
on Windows, possibly generating all the requires wrappers and what not. That sounds like a best thing since sliced bread, and indeed it is.
Discovered via GNPy, thanks to Stefan Melin from Telia (@Sme791) for reporting a test failure on Windows.
Do not wrap a shell script with Python on Windows
This code attempted to produce Python wrappers around native Python code and creating some .bat
files on Windows. The previous commit ported all Python scripts to a native equivalent from setuptools.
Since the only remaining script is a bash one, not a Python code, let's remove that special casing. Those on Windows need Bash for this script anyway, so let them invoke bash path/to/yang2dsdl
directly.
Fix invalid regular expression on Windows
The code (Cc: @mlittlej, #809) was trying to be portable by using the OS-specific path separator in a regular expression. However, on Windows the path separator is a backslash which is a special character in a regular expression.
However, simply wrapping os.pathsep
in re.compile()
would not be the right thing to do here. It is very common to also use paths with Unix-style forward slashes on Windows, especially with portable projects which want to use the same scripts on Unix. Since forward slashes are not allowed in file names on Windows (and they can be used "instead of" backslashes in a lot of contexts), using both is OK on Windows. Anyone using backslashes in, say, Linux, will see a change of behavior, but come on, that would not exactly be the sanest thing to do. Also, YANG disallows a lot of funny characters in module names, so let's be reasonable here.
Of course the best fix here would be to use something like pathlib for path handling, and only apply the regex on actual file names, but I would prefer to use pyang in Windows CI for our project without doing a major refactoring here.
For the "Fix invalid regular expression on Windows" part, I fixed it by re.escape(os.sep)
Hi @jktjkt, hi @fredgan, I fixed some tests, and I added pathlib. What do you think? I am at the IETF Hackathon in London and thought I could do some pyang work.
https://github.com/jktjkt/pyang/pull/1
As I just got informed, @jktjkt only cares about the invalid regular expression on windows which is fixed now. I am interested in moving the scrips into the repository and using entry_points. I guess you can close this PR, and I will create a new one with working tests.