rpms / python-setuptools

Clone
Source Code
GIT

#39 Proof of concept: Validate bundled provides in %check with RPM macros
Closed 3 years ago by churchyard. Opened 3 years ago by churchyard.
rpms/ churchyard/python-setuptools check_vendored  into  master

file modified
+30 -10 
@@ -45,6 +45,29 @@ 
 

  BuildRequires:  python3-rpm-generators
 

  %endif # without bootstrap
 

  

+ # Virtual provides for the packages bundled by setuptools.
 

+ # You can find the versions in setuptools/setuptools/_vendor/vendored.txt
 

+ # We cannot read them at this point, but we can assert their validity in %%check
 

+ %{lua:
 

+   bundled = {
 

+     packaging="16.8",
 

+     pyparsing="2.2.1",
 

+     six="1.10.0",
 

+     appdirs="1.4.3",
 

+   }
 

+ }
 

+ %global bundled_file pkg_resources/_vendor/vendored.txt
 

+ %global bundled %{lua:
 

+   for name in pairs(bundled) do
 

+     print("Provides: bundled(python3dist(" .. name .. ")) = " .. bundled[name] .. "\\n")
 

+   end
 

+ }
 

+ %global bundled_file_expected %{lua:
 

+   for name in pairs(bundled) do
 

+     print(name .. "==" .. bundled[name] .. "\\\\n")
 

+   end
 

+ }
 

+ 

  %description
 

  Setuptools is a collection of enhancements to the Python distutils that allow
 

  you to more easily build and distribute Python packages, especially ones that
 
@@ -53,15 +76,6 @@ 
 

  This package also contains the runtime components of setuptools, necessary to
 

  execute the software that requires pkg_resources.
 

  

- # Virtual provides for the packages bundled by setuptools.
 

- # You can find the versions in setuptools/setuptools/_vendor/vendored.txt
 

- %global bundled %{expand:
 

- Provides: bundled(python3dist(packaging)) = 16.8
 

- Provides: bundled(python3dist(pyparsing)) = 2.2.1
 

- Provides: bundled(python3dist(six)) = 1.10.0
 

- Provides: bundled(python3dist(appdirs)) = 1.4.3
 

- }
 

- 

  %package -n python3-setuptools
 

  Summary:        Easily build and distribute Python 3 packages
 

  Conflicts:      python-setuptools < %{version}-%{release}
 
@@ -142,8 +156,14 @@ 
 

  %endif
 

  

  

- %if %{with tests}
 

  %check
 

+ # Assert our bundling info is correct
 

+ # NB: this is not bash, so <(sort ...) doesn't work
 

+ sort %{bundled_file} > upstream_info
 

+ echo -ne '%{bundled_file_expected}' | sort > downstream_info
 

+ diff -Bbu upstream_info downstream_info
 

+ 

+ %if %{with tests}
 

  # --ignore=pavement.py:
 

  #   pavement.py is only used by upstream to do releases and vendoring, we don't ship it
 

  PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=$(pwd) pytest-%{python3_version} \

We repeatadly forget to update the list of bndled provides.

This is a proof of concept of validating the provides in %check.

It is not very elegant, but if we like the idea, I can macronize most of this, so the usage would only be:

%global bundled_file pkg_resources/_vendor/vendored.txt
%{lua:
  bundled = {
    packaging="16.8",
    pyparsing="2.2.1",
    six="1.10.0",
    appdirs="1.4.3",
  }
}

%package -n python3-setuptools
...
%bundled_provides -f python3dist({name})

%check
...
%validate_bundles

And we could reuse it in setuptools, pip, pipenv...

I've realized this block is not in %check but in %install when run --without tests. I can fix that if we want to explore this idea further.

I prefer this alternative, the code looks simpler and I think it's better to have it inside the spec file rather than in a standalone script.

Can't we get the bundled_count from bundled?

Can there be empty lines/comments in vendored.txt? I'd much rather rely on the diff (ignoring whitespace).

How about diff -Bu to ignore blank lines and whitespace? We don't know what weird vendored files are lurking out there.

First let me say that I really like this idea in general, and I'd love to see this available for use in other packages.

Now for the implementation:

I see that the alternative version is much more robust than this:

  • Converts Python versions to RPM versions (for pre/post releases)
  • Ignores comments, blank lines and white space in the vendored.txt

On the other hand, I'm not sure how we would ship the Python script so that it can be used in other packages, but I suppose it could be done.

Can't we get the bundled_count from bundled?

We can and if this is macronized away from the spec, I would.
However when this is in spec, I'd rather avoid it. The only way to do it is to iterate and count (Lua has no len function for tables not indexed by subsequent integers). HEnce it would make the code even more complicated.

Can there be empty lines/comments in vendored.txt?

Technically, yes. The other PR handles the file more generally. Yes, we can handle this on the diff level. Originally I've compared the sets in Lua: for each line, check if we have it; the len was a check for leftovers. However that don't work (Lua is evaluated when parsing the spec, not when %check is executed), so this entire thing can go away and we can rely on diff. Thanks!

How about diff -Bu...

Sure. diff -Bbu?

On the other hand, I'm not sure how we would ship the Python script so that it can be used in other packages, but I suppose it could be done.

No problem with that, we'll just stick it somewhere into /usr/lib/rpm (like compilall2).

2 new commits added

  • Fixup: Use nicer diff and ignore all whitespace changes (newlines and amount)
  • Fixup: Drop no longer useful bundled_count
3 years ago

I've pushed three fixups here.

1 new commit added

  • Fixup: Make the bunled check part of %check even --without tests
3 years ago

How about diff -Bu...

Sure. diff -Bbu?

Uf, that was an unfortunate typo, I meant -Bw, -w is even stronger than -b, it ignores all white space, which I'd say is what we want.

On the other hand, I'm not sure how we would ship the Python script so that it can be used in other packages, but I suppose it could be done.

No problem with that, we'll just stick it somewhere into /usr/lib/rpm (like compilall2).

Ah, sounds good.

I've pushed three fixups here.

I see you've moved the bundled check into %check but outside of %if %{with tests}. I would argue we want it disabled when tests are disabled. When you're doing a quick scratch build, you don't want to have to comment it out by hand.

Anyway, as for the 2 different versions:

Arguments for the Python implementation

  • Bundled Provides look the same in the spec file
  • Handles pre/post-release versions well
  • Handles comments in vendored.txt
  • Python infrastructure written in Python
  • Can be used to actually generate the provides section in the future

Arguments for the Lua implementation

  • Easier to read code according to @thrnciar (no preference on my part)
  • All handled in macros (no need to ship a Python file)

So as it stands, I'm leaning towards Python. Though some of the concerns could be addressed to to even out the scales.
Edited 3 years ago by torsava

Easier to read code according to @thrnciar

This was posted here, hence it is an argument for the lua/macro/shell implematation.

Easier to read code according to @thrnciar

This was posted here, hence it is an argument for the lua/macro/shell implematation.

it was posted here, but about the alternative version. The comment was:

I prefer this alternative, the code looks simpler and I think it's better to have it inside the spec file rather than in a standalone script.

it was posted here, but about the alternative version. The comment was:

OMG, nope, dyslexia attack. I read it as "I prefer the alternative", as in the other one, but it's actually this alternative. Apologies. I'll edit the balance sheet.
Edited 3 years ago by torsava

recheck

Build succeeded.

Another idea: What if we stick in a bundled.txt file the egg-info / dist-info directory and modify the provides generator to do this? Obviously, we would need to hack around this for wheels, but as a general approach, it makes much more sense. We could even propose this as a standard in upstream.

-1 for that. Let's not put stuff in dist-info unless it's standardized, and let's not open the worm can of standardizing bundling.

Pull-Request has been closed by churchyard
3 years ago
Changes Summary 1
+30 -10
file changed
python-setuptools.spec
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.