Use entry_points for console scripts #833
Conversation
|
Why did you move the scripts from |
|
Using
With entry points, the behaviour is the same as other python packages. as soon the virtual env is activated, the commands are in the $PATH available. |
|
so what are the steps to run pyang locally? |
You could also do So if someone wants to work on the repo I would recommend: |
This doesn't work for me. Where is the script |
|
The script itself is still at the same place in the script directory but if you are using venv in the Example with a fresh docker container: |
|
Right, that was your third option above, using venv. I got that to work. But I was thinking about the second otpion with just |
|
If you run it as a normal user I assume it will be in |
|
If pip installs the script in a location not containing in $PATH, it should get a warning printed: |
|
Ok, thanks. Another question, why is the script file called |
|
I renamed |
|
Ok. Could you rebase you branch on master and update README.md with new install instructions? |
|
I updated the README and added aliases to the |
|
Thanks! Github says that the branch cannot be rebased due to conflicts. I haven't checked what the conflicts are yet... |
|
I don't see any conflicts |
|
Have you tried to rebase your branch against the latest mbj4668:master? I tried it locally and hit this conflict in Having resolved this conflict (taking the |
|
Note: I didn't mean to imply that the right way to resolve the conflict is necessarily just to take the 23c51f1 changes. It appears that ubaumann:master (from which you presumably branched ubaumann:windows) is three commits behind mbj4668:master. One of these commits (17aa7ad) made these changes to |
|
(I'm not sure why GitHub says |
The current approach is not recoomended [1], 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 [2]. 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 for reporting a test failure on Windows. [1] https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#scripts [2] https://setuptools.pypa.io/en/latest/userguide/quickstart.html#entry-points-and-automatic-script-creation
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.
The code 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. Fixes: dad5c68 Fix issue #809: revision-date parsed wrong if >1 @ in path
Dependenciy problem between required version of importlib-metadata in flake8 and virtualenv
|
I rebased locally and force-pushed the new history. I hope now it works |
|
@ubaumann, I'd like to request that the Looking at your branch I think that it would work to create soft links in ...but I don't really see why the scripts need to be moved in their entirety to What do you think? |
| export MANPATH="$PWD/man:$MANPATH" | ||
| export PYTHONPATH="$PWD:$PYTHONPATH" | ||
| export YANG_MODPATH="$PWD/modules:$YANG_MODPATH" | ||
| export PYANG_XSLT_DIR="$PWD/xslt" | ||
| export PYANG_RNG_LIBDIR="$PWD/schema" | ||
| export W="$PWD" | ||
|
|
||
| if ! pyang --help &> /dev/null |
There was a problem hiding this comment.
What exactly is this doing? If it's checking whether the pyang command already works then perhaps this could be done more directly? Perhaps it would be better to check this first, before changing PYTHONPATH etc., so the script either does nothing or does everything?
@wlupton Sure, it would be possible to add. On the other hand, somewhen it is to stop supporting legacy stuff. Can a maintainer tell me what I need to do so this PR can be merged? |
|
The only reason we still support 2.7 is that we simply didn't have had any reason for not doing so. You wrote earlier that you renamed the script due to import issues with different versions of python. Would it help if we removed support for 2.7? People rely on the the script |
|
@mbj4668 I am not sure when exactly the import behaviour of absolute and relative changed, but it differs between 2.7 and newer python 3 versions. What is the last version pyang needs to support? 3.7 (end of life: 2023-06-27) or 3.8? I can remove older versions in the tests and clean up some stuff. I think it should be possible to make the imports work with the name |
|
Yes I think 3.7 makes sense. |
* Update python-version in github tests workflow * Remove python2 * Clean up code * Update test 16 expected output * Create bin alias scripts for cli tools
|
I removed the old python versions and created the endpoint scripts in the
Tests are running: https://github.com/ubaumann/pyang/actions/runs/3907015157 Now the following ways should work.
What do you think? |
|
I think it looks really good, thank you for doing this! I'll have a more detailed look later this week. Perhaps this needs to be a new major version (3.0). |
Based on #820, I continued the work to remove the python files in the /bin folder and use entry_points for the installation
Moving the scripts brings a couple of advantages and makes installing it on different platforms more accessible. Also, the scripts are now in the library and can be imported from other python scripts. I could also fix all the tests.