Bug 1924918 - Review Request: reprotest - Build packages and check them for reproducibility
Summary: Review Request: reprotest - Build packages and check them for reproducibility
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-03 21:27 UTC by Frédéric Pierret
Modified: 2021-02-18 14:35 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-02-18 14:35:32 UTC
Type: Bug
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Frédéric Pierret 2021-02-03 21:27:10 UTC
# reprotest:
- Description: reprotest builds the same source code twice in different environments, and then checks the binaries produced by each build for differences. If any are found, then diffoscope (or if unavailable then diff) is used to display them in detail for later analysis.

- GIT: https://github.com/fepitre/fedora-reprotest
- SPEC: https://raw.githubusercontent.com/fepitre/fedora-reprotest/master/reprotest.spec
- COPR BUILDS: https://copr.fedorainfracloud.org/coprs/fepitre/fedora/build/1938423/
- SRPM RAWHIDE: https://download.copr.fedorainfracloud.org/results/fepitre/fedora/fedora-rawhide-x86_64/01938423-reprotest/reprotest-0.7.16-1.fc34.src.rpm

This packages depends on python-rstr and disorderfs not packaged yet in Fedora.

## python-rstr:
- Description: Generate random strings in Python
- GIT: https://github.com/fepitre/fedora-python-rstr
- SPEC: https://raw.githubusercontent.com/fepitre/fedora-python-rstr/master/python-rstr.spec
- SRPM: https://download.copr.fedorainfracloud.org/results/fepitre/fedora/fedora-rawhide-x86_64/01938416-python-rstr/python-rstr-2.1.0-1.fc34.src.rpm


## disorderfs:
- Description: FUSE filesystem that introduces non-determinism

- GIT: https://github.com/fepitre/fedora-disorderfs
- SPEC: https://raw.githubusercontent.com/fepitre/fedora-disorderfs/master/disorderfs.spec
- SRPM: https://download.copr.fedorainfracloud.org/results/fepitre/fedora/fedora-rawhide-x86_64/01938412-disorderfs/disorderfs-0.5.10-1.fc34.src.rpm.


reprotest and disorderfs are developed as part of the "reproducible builds" Debian project. I've recently implemented RPM support into reprotest.


This is my first packages for Fedora and I am seeking a sponsor. I would be happy to split the two dependencies as separate tickets if needed.

Comment 1 Zbigniew Jędrzejewski-Szmek 2021-02-04 09:06:28 UTC
For discordfs:

> GPL-3+
The tag in Fedora is GPLv3+, see https://fedoraproject.org/wiki/Licensing:Main#Software_License_List.

No %check? I see disorderfs has tests upstream.

> Requires:       bc
Why bc? The program is a C++ binary, does it call out to bc?

> %files
Add '%license COPYING'

> BuildArch:      x86_64
Oh, why? Is the program non-portable?

Also add 'BuildRequires: make'.

Comment 2 Zbigniew Jędrzejewski-Szmek 2021-02-04 09:07:02 UTC
Please open a separate review ticket for each component.

Comment 3 Zbigniew Jędrzejewski-Szmek 2021-02-04 09:16:49 UTC
For rstr:

> License:        None
It needs to be one of the license under https://fedoraproject.org/wiki/Licensing:Main#Software_License_List.

> %if %{with tests}
> BuildRequires:  python3dist(rstr)
> %endif

That's be super annoying. In particular it'd complicate bootstrapping to each next python version.
Instead of doing that, just add PYTHONPATH=%buildroot%{python3_sitelib} for the tests.

Don't repeat the description text. Instead do:
%global _description %{expand:
<the text here>}

%description %_description

%description -n python%{python3_pkgversion}-%{pypi_name} %_description

> It has no dependencies outside the standard library, and should
> be compatible with Python 3.

Drop this. There's nothing except python 3 and users don't need to care about dependencies,
packaging metadata takes care of this for them ;)

Comment 4 Frédéric Pierret 2021-02-06 09:24:13 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> Please open a separate review ticket for each component.

I've open separate tickets: https://bugzilla.redhat.com/show_bug.cgi?id=1925759 and https://bugzilla.redhat.com/show_bug.cgi?id=1925758.

Comment 5 Frédéric Pierret 2021-02-06 09:29:12 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> For discordfs:
> 
> > GPL-3+
> The tag in Fedora is GPLv3+, see
> https://fedoraproject.org/wiki/Licensing:Main#Software_License_List.

Fixed thank you.

> No %check? I see disorderfs has tests upstream.

I've added check with respect to current upstream tests. Unfortunately, it works locally but not on COPR and probably not into Koji because tests are using fuse and it fails due to that. The current workaround is to "|| true" but I admin I would love any better suggestion if we have a way to handle fuse tests on build infra.
 
> > Requires:       bc
> Why bc? The program is a C++ binary, does it call out to bc?

bc is only used for tests. I've fixed dependency to BR.

> > %files
> Add '%license COPYING'

Added.
 
> > BuildArch:      x86_64
> Oh, why? Is the program non-portable?

It's a mistake. I've corrected it.
 
> Also add 'BuildRequires: make'.

Added too.

If needed you can answer on the corresponding new ticket. I've answered here to make you easier to read my responses.

Comment 6 Zbigniew Jędrzejewski-Szmek 2021-02-09 07:37:05 UTC
> Unfortunately, it works locally but not on COPR and probably not into Koji because tests are using fuse and it fails due to that. The current workaround is to "|| true" but I admin I would love any better suggestion if we have a way to handle fuse tests on build infra.

OK. Later on you might want to create a post-build test using a VM.
https://docs.fedoraproject.org/en-US/ci/ in principle has some docs, but I know
from experience that it is very hard to get something started using those docs.
So let's leave out functional tests out for now.

I'll look at the other tickets first.

Comment 7 Frédéric Pierret 2021-02-09 17:57:18 UTC
I've updated also sources verification signature. Spec is updated and the github repository and latest working build here: https://download.copr.fedorainfracloud.org/results/fepitre/fedora/fedora-rawhide-x86_64/01955981-reprotest/. For your information %check section would be added soon once my merge request https://salsa.debian.org/reproducible-builds/reprotest/-/merge_requests/13/diffs will be merged. It allows to handle upstream tests nicely under Fedora. I plan to add Fedora specific tests too on upstream.

Comment 8 Zbigniew Jędrzejewski-Szmek 2021-02-13 18:41:27 UTC
You added the signature file, but it's not used for anything…

Please add URL: field with a link to the upstream project home page.

Is glibc-all-langpacks really necessary? That's a lot of data. 

+ package name is OK
+ license is acceptable (GPLv3+)
+ license is specified correctly
+ builds and installs OK
+ R/P/BR look OK

rpmlint:
rpmlint reprotest-0.7.16-3.fc34.noarch.rpm reprotest-0.7.16-3.fc34.src.rpm
reprotest.noarch: W: spelling-error Summary(en_US) reproducibility -> reprehensibility (nice one ;))
reprotest.noarch: W: no-url-tag (see above)
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/VirtSubproc.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/adt_testbed.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/adtlog.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/__init__.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/arch.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/debian.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/guix.py
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-chroot
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-lxc
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-lxd
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-null
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-qemu
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-schroot
reprotest.noarch: E: incorrect-fsf-address /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-ssh
reprotest.noarch: W: no-manual-page-for-binary reprotest
Hmm, IIRC, Debian requires a man page for every package, so there should be one somewhere. Please also add it here if possible.

2 packages and 0 specfiles checked; 14 errors, 12 warnings.

Package is APPROVED.

Please note that the spec file in dist-git is the canonical version.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_maintenance_and_canonicity says:
> Maintainers MUST expect that other maintainers and automated tooling will make changes to their packages,
> potentially without communicating prior to doing so (though communication is always encouraged). If some
> maintainers are also attempting to keep copies of a spec in an outside repository, they MUST be prepared
> to merge changes made to the spec in Fedora’s repository, and MUST NOT overwrite those changes with a
> copy from an external repository

i.e. if you want to keep the spec file in the upstream project, that is OK, but occasionally you'll need
to move stuff manually to the version in upstream.

Comment 9 Frédéric Pierret 2021-02-14 15:59:42 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #8)
> You added the signature file, but it's not used for anything…
> 
> Please add URL: field with a link to the upstream project home page.

Yes sorry I've literally forgot it while fixing stuff.

> Is glibc-all-langpacks really necessary? That's a lot of data. 

What's needed in reprotest is several locales which are randomly changed for reproducible tests and I've not found any other alternative to this big package? Any other clue?  

> + package name is OK
> + license is acceptable (GPLv3+)
> + license is specified correctly
> + builds and installs OK
> + R/P/BR look OK
> 
> rpmlint:
> rpmlint reprotest-0.7.16-3.fc34.noarch.rpm reprotest-0.7.16-3.fc34.src.rpm
> reprotest.noarch: W: spelling-error Summary(en_US) reproducibility ->
> reprehensibility (nice one ;))
> reprotest.noarch: W: no-url-tag (see above)
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/VirtSubproc.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/adt_testbed.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/adtlog.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/__init__.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/arch.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/debian.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/lib/system_interface/guix.py
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-chroot
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-lxc
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-lxd
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-null
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-qemu
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-schroot
> reprotest.noarch: E: incorrect-fsf-address
> /usr/lib/python3.9/site-packages/reprotest/virt/autopkgtest-virt-ssh
> reprotest.noarch: W: no-manual-page-for-binary reprotest
> Hmm, IIRC, Debian requires a man page for every package, so there should be
> one somewhere. Please also add it here if possible.
> 
> 2 packages and 0 specfiles checked; 14 errors, 12 warnings.
> 
> Package is APPROVED.
> 
> Please note that the spec file in dist-git is the canonical version.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> #_spec_maintenance_and_canonicity says:
> > Maintainers MUST expect that other maintainers and automated tooling will make changes to their packages,
> > potentially without communicating prior to doing so (though communication is always encouraged). If some
> > maintainers are also attempting to keep copies of a spec in an outside repository, they MUST be prepared
> > to merge changes made to the spec in Fedora’s repository, and MUST NOT overwrite those changes with a
> > copy from an external repository
> 
> i.e. if you want to keep the spec file in the upstream project, that is OK,
> but occasionally you'll need
> to move stuff manually to the version in upstream.

Yes we plan to also use separate branch on upstream to maintain this spec. Notably, for possible OpenSUSE community to use it.

Comment 10 Zbigniew Jędrzejewski-Szmek 2021-02-14 20:20:18 UTC
> > Is glibc-all-langpacks really necessary? That's a lot of data. 
> 
> What's needed in reprotest is several locales which are randomly changed for
> reproducible tests and I've not found any other alternative to this big
> package? Any other clue?

You could do something like
Requires: glibc-langpack-fr
Requires: glibc-langpack-es
Requires: glibc-langpack-ru
Requires: glibc-langpack-kk
Requires: glibc-langpack-zh
Requires: glibc-langpack-en

If the list of locales used by reprotest doesn't change often, that'd be OK. But
if you'd have to adapt it periodically, than I don't think it makes sense.
glibc-all-langpacks is 230MB, it's not end of the world.

> Yes we plan to also use separate branch on upstream to maintain this spec.
> Notably, for possible OpenSUSE community to use it.

That'll only work as long if the two packaging standards are exactly in alignment.
I think it'll be more work than it's worth with proxying the changes three ways.
But that's just an opinion…

Comment 11 Mohan Boddu 2021-02-17 14:30:04 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/reprotest


Note You need to log in before you can comment on or make changes to this bug.