Skip to content

Comments

setup: sanitize package version#3862

Merged
gpotter2 merged 2 commits intosecdev:masterfrom
rjarry:master
Feb 13, 2023
Merged

setup: sanitize package version#3862
gpotter2 merged 2 commits intosecdev:masterfrom
rjarry:master

Conversation

@rjarry
Copy link
Contributor

@rjarry rjarry commented Jan 20, 2023

Currently, when installing from a non-tagged git archive version we get an error from pkg_resources:

pkg_resources.extern.packaging.version.InvalidVersion: Invalid version: 'git-archive.dev95ba5b8504'

This version does not comply with the PEP 440 standard.

Update the parsing of git-archive %(describe) placeholder by adding multiple safeguards for computing scapy.VERSION (the first successful method is used in priority):

  1. If the SCAPY_VERSION env var is defined, use it. This will allow downstream packaging distros to force a specific version even if they store scapy in a different repository using a different git tag scheme.

  2. If the scapy/VERSION file exists, use its contents.

  3. Try to parse a tag from a git archive %(describe) placeholder. If the git archive was not made on a tag, use the commit timestamp to convert it to a date YYYY.MM.DD which is PEP 440 compatible.

  4. Try to parse the version from git describe.

  5. Use the last modification of scapy/__init__.py and generate a date YYYY.MM.DD which is PEP 440 compatible.

  6. Return 0.0.0

Do not try to generate the scapy/VERSION file when importing scapy anymore but generate it by overriding the sdist command in setup.py and write it to the temp folder used for the source archive generation.

Update unit tests to ensure that order of priority is enforced.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2162667

Currently, when installing from a non-tagged git archive version we get
an error from pkg_resources:

pkg_resources.extern.packaging.version.InvalidVersion: Invalid version:
'git-archive.dev95ba5b8504'

This version does not comply with the PEP 440 standard.

Update the parsing of git-archive %(describe) placeholder by adding
multiple safeguards for computing scapy.VERSION (the first successful
method is used in priority):

1) If the SCAPY_VERSION env var is defined, use it. This will allow
   downstream packaging to force a specific version even if they store
   scapy in a different repository using a different git tag scheme.

2) If the scapy/VERSION file exists, use its contents.

3) Try to parse a tag from a git archive %(describe) placeholder. If the
   git archive was not made on a tag, use the commit timestamp to
   convert it to a date YYYY.MM.DD which is PEP 440 compatible.

4) Try to use git describe to generate a tag.

5) Use the last modification date of scapy/__init__.py and generate
   a date YYYY.MM.DD which is PEP 440 compatible.

6) Return 0.0.0

Do not try to generate the scapy/VERSION file when importing scapy
anymore but generate it by overriding the sdist command in setup.py and
write it to the temp folder used for the source archive generation.

Update unit tests to ensure that order of priority is enforced.

Link: https://peps.python.org/pep-0440/
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2162667
Signed-off-by: Robin Jarry <rjarry@redhat.com>
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #3862 (8765aa6) into master (3ce66d1) will decrease coverage by 1.51%.
The diff coverage is 54.05%.

@@            Coverage Diff             @@
##           master    #3862      +/-   ##
==========================================
- Coverage   51.93%   50.43%   -1.51%     
==========================================
  Files         190      227      +37     
  Lines       43195    52757    +9562     
==========================================
+ Hits        22435    26610    +4175     
- Misses      20760    26147    +5387     
Impacted Files Coverage Δ
scapy/__init__.py 64.78% <54.05%> (+2.97%) ⬆️
scapy/utils6.py 29.25% <0.00%> (-1.00%) ⬇️
scapy/arch/windows/__init__.py 62.21% <0.00%> (-0.57%) ⬇️
scapy/layers/l2.py 43.07% <0.00%> (-0.25%) ⬇️
scapy/layers/tls/crypto/h_mac.py 78.46% <0.00%> (ø)
scapy/contrib/bier.py 100.00% <0.00%> (ø)
scapy/layers/tls/automaton.py 15.78% <0.00%> (ø)
scapy/layers/tls/crypto/prf.py 29.37% <0.00%> (ø)
scapy/layers/tls/automaton_srv.py 22.56% <0.00%> (ø)
... and 42 more

gpotter2
gpotter2 previously approved these changes Jan 23, 2023
Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

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

This LGTM. I think that @guedou is best suited to give some feedback on this :p Thanks !

@rjarry
Copy link
Contributor Author

rjarry commented Feb 8, 2023

Hey guys, gentle ping to get progress. @guedou do you have some remarks on these changes?

Copy link
Member

@guedou guedou left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@guedou
Copy link
Member

guedou commented Feb 9, 2023

What is the command that triggers this issue? I could not reproduce it with pip install git+https://github.com/secdev/scapy, or

$ python setup.py build
$ python setup.py sdist
$ pip install dist/scapy-2.5.0.dev27.tar.gz

@gpotter2
Copy link
Member

gpotter2 commented Feb 9, 2023

If you clone scapy with --depth=1and use pip and setuptools latest version, I was able to reproduce.

@gpotter2
Copy link
Member

So all unit tests are currently failing because of this 😆

@gpotter2 gpotter2 merged commit 915fe2d into secdev:master Feb 13, 2023
@rjarry
Copy link
Contributor Author

rjarry commented Feb 13, 2023

Hehe thanks!

muttiopenbts pushed a commit to muttiopenbts/scapy that referenced this pull request Feb 21, 2023
* setup: sanitize package version

Currently, when installing from a non-tagged git archive version we get
an error from pkg_resources:

pkg_resources.extern.packaging.version.InvalidVersion: Invalid version:
'git-archive.dev95ba5b8504'

This version does not comply with the PEP 440 standard.

Update the parsing of git-archive %(describe) placeholder by adding
multiple safeguards for computing scapy.VERSION (the first successful
method is used in priority):

1) If the SCAPY_VERSION env var is defined, use it. This will allow
   downstream packaging to force a specific version even if they store
   scapy in a different repository using a different git tag scheme.

2) If the scapy/VERSION file exists, use its contents.

3) Try to parse a tag from a git archive %(describe) placeholder. If the
   git archive was not made on a tag, use the commit timestamp to
   convert it to a date YYYY.MM.DD which is PEP 440 compatible.

4) Try to use git describe to generate a tag.

5) Use the last modification date of scapy/__init__.py and generate
   a date YYYY.MM.DD which is PEP 440 compatible.

6) Return 0.0.0

Do not try to generate the scapy/VERSION file when importing scapy
anymore but generate it by overriding the sdist command in setup.py and
write it to the temp folder used for the source archive generation.

Update unit tests to ensure that order of priority is enforced.

Link: https://peps.python.org/pep-0440/
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2162667
Signed-off-by: Robin Jarry <rjarry@redhat.com>

* Reject invalid git tags

---------

Signed-off-by: Robin Jarry <rjarry@redhat.com>
Co-authored-by: gpotter2 <10530980+gpotter2@users.noreply.github.com>
@gpotter2 gpotter2 added this to the 2.6.0 milestone Nov 28, 2023
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.

3 participants