Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pins dependency version and fix mininet install with pip #41

Merged
merged 3 commits into from
Aug 7, 2019

Conversation

jadinm
Copy link
Collaborator

@jadinm jadinm commented Jul 26, 2019

Closes Issue #39

This pins versions of dependencies in the setup.py.
It also installs the Mininet dependencies if not already present.

This implies changes in the install scripts: they are moved inside the ipmininet library so that they can be used by the setup.py file. The documentation is also updated.

Some points to discuss:

  1. The chosen versions for each dependency
  2. The Mininet dependencies (that are not packages) are installed in $HOME/mininet_dependencies. Maybe another location would be more suitable.
  3. The syntax mininet @ git+https://github.com/mininet/mininet@2.3.0d6 is only supported from pip 19. There is another way of installing a dependency from github but it is deprecated and it requires extra arguments when running pip. Still, this might not be the best solution.

@oliviertilmans
Copy link
Member

1. The chosen versions for each dependency

Special care needs to be taken with pinned deps (i.e., '==') so that ipmininet does not prevent those packages from being upgraded in the future...

2. The Mininet dependencies (that are not packages) are installed in $HOME/mininet_dependencies. Maybe another location would be more suitable.

I'd personally for for /opt for that, and then either symlinking the exec in /usr/bin or modifying $PATH (possibly using /etc/environment or ...)

3. The syntax `mininet @ git+https://github.com/mininet/mininet@2.3.0d6` is only supported from pip 19. There is another way of installing a dependency from github but it is [deprecated](https://github.com/pypa/pip/issues/4187) and it requires extra arguments when running pip. Still, this might not be the best solution.

I am certain I used a similar syntax without issues a few years ago (possibly with the now deprecated hook). An alternative would be to auto-detect whether mininet is installed in your install script and do it 'manually' (i.e., side-stepping pip as we're not using pypi for that dep. anyway).

@jadinm
Copy link
Collaborator Author

jadinm commented Jul 30, 2019

1. The chosen versions for each dependency

Special care needs to be taken with pinned deps (i.e., '==') so that ipmininet does not prevent those packages from being upgraded in the future...

I only set mininet with a strict version.
I chose 2.3.0d6 because 2.2.2 (the last stable version) does not support Python3.

2. The Mininet dependencies (that are not packages) are installed in $HOME/mininet_dependencies. Maybe another location would be more suitable.

I'd personally for for /opt for that, and then either symlinking the exec in /usr/bin or modifying $PATH (possibly using /etc/environment or ...)

Ok

3. The syntax `mininet @ git+https://github.com/mininet/mininet@2.3.0d6` is only supported from pip 19. There is another way of installing a dependency from github but it is [deprecated](https://github.com/pypa/pip/issues/4187) and it requires extra arguments when running pip. Still, this might not be the best solution.

I am certain I used a similar syntax without issues a few years ago (possibly with the now deprecated hook). An alternative would be to auto-detect whether mininet is installed in your install script and do it 'manually' (i.e., side-stepping pip as we're not using pypi for that dep. anyway).

Yes, with the deprecated argument, you could set a dependency_links parameter with github links from pip 10. And actually, the git link directly in the install_require parameter works since pip 18.1.

I would rather not use the manual strategy. This might prevent someone from using specific parameters of pip (e.g., "--target <dir>") and it is also a bit strange not to have mininet as explicit python dependence.

@oliviertilmans
Copy link
Member

oliviertilmans commented Jul 31, 2019 via email

@jadinm jadinm force-pushed the improve-pip-setup branch 2 times, most recently from 6a7beae to 46a1bd6 Compare July 31, 2019 14:00
@jadinm
Copy link
Collaborator Author

jadinm commented Jul 31, 2019

Good point. I can you add version check in the setup script to warn that it will fail
to install mininet and maybe print a manual fallback to be executed by the user?
Beside updating pip itsel ofc (which might be managed by the distribution hence
lag behind a few versions). Such issue is temporary anyway and we may just ignore
it altogether past documentation.

Ok, I modified the setup.py file to take into account the pip version.
I cannot check that users added the "--process-dependency-links" parameter because it is handled internally by pip and I don't have access to that.
So, I printed a warning (see below) but pip won't show it unless users add a "-v" to the pip call.

Nevertheless, this improves the PR since it is does no longer require to update pip.

# Get back Pip version
try:
    version = parse_version(require("pip")[0].version)
except IndexError:
    version = parse_version("0")
    print("We cannot find the version of pip."
          "We assume that is it inferior to 18.1.")

if version >= parse_version("18.1"):
    install_requires.append('mininet @ git+https://github.com/mininet/mininet@{ver}'
                            .format(ver=MININET_VERSION))
else:
    print("You should run pip with --process-dependency-links to install all dependencies")
    install_requires.append('mininet=={ver}'.format(ver=MININET_VERSION))
    dependency_links.append('git+https://github.com/mininet/mininet@{ver}#egg=mininet-{ver}'
                            .format(ver=MININET_VERSION))

This implies changes in the install scripts: they are moved inside
the ipmininet library so that they can be used by the `setup.py` file.
The documentation is also updated.
@jadinm jadinm merged commit 6f96f4f into cnp3:master Aug 7, 2019
@jadinm jadinm deleted the improve-pip-setup branch November 20, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants