-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
virtualenv: Bump version to 20.0.13 and add dependencies #6365
Conversation
Notifying maintainers: |
This comment has been minimized.
This comment has been minimized.
The commit message needs to be modified. Ideally this would be 2 commits. One for the new port, which needs to be mentioned in this commit. And the other commit for "py-virtualenv" not "virtualenv" as the current CM calls it. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me there are some issues with the dependencies that need to get sorted out before this can be merged (and a few minor details)
701afa5
to
afe0926
Compare
The new version
|
This comment has been minimized.
This comment has been minimized.
python/py-virtualenv/Portfile
Outdated
depends_lib-append port:py${python.version}-appdirs \ | ||
port:py${python.version}-distlib \ | ||
port:py${python.version}-filelock \ | ||
port:py${python.version}-setuptools \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't depend on setuptools 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't all the python ports depend on setuptools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope 👍 only old-style setup.py builds (which are not supported anymore) so hope we don't do that... if you build via pip or pep-517 this should not be needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mh, could you please link me an example of a portfile using PEP-517 or pip
to install? I tried to grep around but I didn't find anything useful... Or it's automatic with the PG?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do build with setup.py
so the build- and runtime dependency on setuptools
is needed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setuptools does not support this behaviour anymore, other than backwards compatible way, so you might probably want to change to pip and wheels 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
virtualenv itself uses pep 517 and 518 standards to build itself, so I don't think you'll be able to build without provisioning first the build system per pep-518
@@ -46,7 +66,7 @@ if {${name} ne ${subport}} { | |||
post-destroot { | |||
set docdir ${prefix}/share/doc/${subport} | |||
xinstall -d ${destroot}${docdir}/docs/changelog | |||
xinstall -m 0644 -W ${worksrcpath} README.rst LICENSE.txt AUTHORS.txt \ | |||
xinstall -m 0644 -W ${worksrcpath} README.md LICENSE \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this file needed? why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a previous review @reneeotten made me keep these files. :)
3089a39
to
feda1e9
Compare
I think this is quite important, since the current version has bugs with subprocess? Can we merge this and then, as soon as 20.0.6 will come out, I'll update again? |
I'd wait for Monday, pypa/virtualenv#1711 will land this weekend with |
Travis Build #10695 Errored. Lint results
Port py38-appdirs success on xcode10.3. Log The build timed out. |
Travis Build #10741 Errored. Lint results
Port py38-appdirs success on xcode10.3. Log The build timed out. |
Also adds dependencies needed for v20.0.5
Ok, I updated at the last version available. :). Please merge this since the available version on Macports is bugged. |
This is now ready to go 👍 |
@reneeotten your requested changes are outdated. Can you have a look at them to confirm this merge is now OK? |
okay, I will take care of getting this merged. There are still a few minor points left (e.g., |
@reneeotten Ehi, I think it's ok even if I believe that this could land now and with another PR then you could upload setuptools. Otherwise with my and your lags, we will never merge it :) |
there were still some issues regarding dependencies with this PR, but I'm almost done with it. There is no need to update |
@reneeotten Ok! Thanks for the support! |
I just committed your new/updated ports with a few modifications to make sure everything works correctly. Thanks again for the update and sorry that it took so long this time get this committed! |
So we can close this PR I guess. Thanks to everybody for helping me! |
Add py-distlib port that is a depenency of virtualenv
Fix this bug: pypa/virtualenv#1581
Description
Type(s)
Tested on
macOS 10.15.2
Xcode 11.3.1
Verification
Have you
port lint
?sudo port test
?sudo port -vst install
?