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

distutils_install_for testing install proper egg-info, dev-python/cryptography fix test on first install #2746

Closed
wants to merge 1 commit into from

Conversation

MathyV
Copy link
Contributor

@MathyV MathyV commented Nov 4, 2016

Currently distutils_install_for_testing does not install the complete
egg-info into ${TEST_DIR}. This was first noticed by W. Trevor King and
reported in bug #524322. Based on info found in the related upstream
setuptools bug I added the necessary call to setuptools to create the
complete egg_info.

Without this certain packages (like cryptography) fail during testing
because they use introspection but the metadata isn't available.

Gentoo-Bug: https://bugs.gentoo.org/show_bug.cgi?id=595272
Gentoo-Bug: https://bugs.gentoo.org/show_bug.cgi?id=524322

mkdir -p "${libdir}" || die
esetup.py "${add_args[@]}" "${@}"
esetup.py "${add_args_egg[@]}"
esetup.py "${add_args_install[@]}" "${@}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to pass both in a single call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than for the sake of clarity... no I guess?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then plus combine that into a single array, and a single call. Two arrays don't make it more readable, and remember that parameters to command also influence behavior of other commands in distutils.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mgorny mgorny self-assigned this Nov 5, 2016
@mgorny mgorny added enhancement assigned PR successfully assigned to the package maintainer(s). labels Nov 5, 2016
@mgorny
Copy link
Member

mgorny commented Nov 5, 2016

CC @gentoo/python.

@djc
Copy link
Contributor

djc commented Nov 5, 2016

LGTM.

@floppym
Copy link
Contributor

floppym commented Nov 5, 2016

I'm a bit confused as to why esetup.py install does not copy/create the egg-info. But if this fixes the problem, I suppose that's good enough for me.

@MathyV
Copy link
Contributor Author

MathyV commented Nov 6, 2016

@floppym the install puts in some egg-info but not everything and for some reason this seems to be desired behaviour...

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM now. However, you need to follow the usual procedure with ml review before committing this.

Furthermore, it would be great if you could figure out what exactly is different here and why do we have to do that.

@mgorny mgorny assigned MathyV and unassigned mgorny Nov 6, 2016
…t install

On first install the test would fail because it uses introspection but the
egg-info was not available. On second install it would work because introspection
would (wrongly) inspect the system installed package.

Added distutils_install_for_testing to have a complete install available for
testing.

Gentoo-Bug: https://bugs.gentoo.org/show_bug.cgi?id=595272

Package-Manager: portage-2.3.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s).
Projects
None yet
4 participants