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

python3Packages.packageurl-python: init at 0.9.4 #116325

Merged
merged 2 commits into from
Mar 20, 2021

Conversation

armijnhemel
Copy link
Contributor

Motivation for this change

packageurl (AKA purl) is an upcoming standard to describe packages in a more uniform way across operating systems and Linux distributions

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@r-rmcgibbo
Copy link

r-rmcgibbo commented Mar 14, 2021

Result of nixpkgs-review pr 116325 at 4706bf95 run on x86_64-linux 1

2 packages built successfully:

Result of nixpkgs-review pr 116325 at 4706bf95 run on aarch64-linux 1

2 packages built successfully:

sha256 = "0mpvj8imsaqhrgfq1cxx16flc5201y78kqa7bh2i5zxsc29843mx";
};

pythonImportsCheck = [ "packageurl" ];
Copy link
Member

@dotlambda dotlambda Mar 14, 2021

Choose a reason for hiding this comment

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

Suggested change
pythonImportsCheck = [ "packageurl" ];
checkInputs = [ pytestCheckHook ];
pythonImportsCheck = [ "packageurl" ];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, added

Copy link
Member

@dotlambda dotlambda Mar 14, 2021

Choose a reason for hiding this comment

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

You should also try whether the tests run successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Loving it that people are giving conflicting information. Very helpful :-/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Loving it that people are giving conflicting information. Very helpful :-/

To be a bit more constructive: the previous time I submitted it I was told to include what I included if I was not able to run tests. I am not able to run the tests at this point.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this was more about the missing argument pytestCheckHook.
If you post the output, we can help figure out how to make the tests run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will need some help here. Note: I am not on NixOS, but Fedora. It has been 15 years since I did any serious work with Nix so my skills are very rusty.

Copy link
Member

Choose a reason for hiding this comment

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

I get

Executing pytestCheckPhase
============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-6.1.2, py-1.9.0, pluggy-0.13.1
rootdir: /build/packageurl-python-0.9.4, configfile: setup.cfg
collected 258 items

tests/test_packageurl.py .....................................           [ 14%]
tests/contrib/test_get_path_segments.py ...                              [ 15%]
tests/contrib/test_purl2url.py ..                                        [ 16%]
tests/contrib/test_url2purl.py ......................................... [ 32%]
........................................................................ [ 60%]
........................................................................ [ 87%]
...............................                                          [100%]

============================= 258 passed in 0.84s ==============================
Finished executing pytestCheckPhase

So everything seems to work.

@SuperSandro2000
Copy link
Member

Please squash the review commit together as you can't do this from the github web ui without also squashing the maintainer add commit.

sha256 = "0mpvj8imsaqhrgfq1cxx16flc5201y78kqa7bh2i5zxsc29843mx";
};

pythonImportsCheck = [ "packageurl" ];
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this was more about the missing argument pytestCheckHook.
If you post the output, we can help figure out how to make the tests run.

pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved
@dotlambda dotlambda mentioned this pull request Mar 14, 2021
10 tasks
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 116325 run on x86_64-linux 1

2 packages built:
  • python38Packages.packageurl
  • python39Packages.packageurl

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

python38Packages.packageurl:

warning: no-python-tests
Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-python-tests.md

dotlambda added a commit to dotlambda/nixpkgs that referenced this pull request Mar 15, 2021
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

The only thing left to be done is squashing the last two commits into the second one and correct the commit message.

@dotlambda dotlambda changed the title python3Packages.packageurl: init at 0.9.4 python3Packages.packageurl-python: init at 0.9.4 Mar 17, 2021
@armijnhemel
Copy link
Contributor Author

grrr, sorry, I thought I had changed the commit message

packageurl: add checkInputs as suggested by review

packageurl -> packageurl-python

packageurl-python: add missing dependency
@dotlambda dotlambda merged commit b1de2b4 into NixOS:master Mar 20, 2021
@armijnhemel armijnhemel deleted the package_url branch March 21, 2021 12:19
dotlambda added a commit that referenced this pull request Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants