[devtools,3/3] makerepropkg: support checking multiple split packages

Message ID 20191216204609.111647-4-eschwartz@archlinux.org
State Accepted
Headers show
Series makerepropkg improvements | expand

Commit Message

Emil Velikov via arch-projects Dec. 16, 2019, 8:46 p.m. UTC
By specifying multiple package files, we assume they are all from the
same PKGBUILD, and try to check them all against the produced artifacts.
Since the buildinfo should be comparable for all of them, we simply use
the first one passed on the command line.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 doc/makerepropkg.1.asciidoc |  8 ++++++--
 makerepropkg.in             | 40 +++++++++++++++++++++----------------
 2 files changed, 29 insertions(+), 19 deletions(-)

Comments

Emil Velikov via arch-projects Dec. 16, 2019, 8:55 p.m. UTC | #1
Please adjust zsh_completions.in to take multiple packages in _makerepropkg_args
Emil Velikov via arch-projects Dec. 19, 2019, 9:19 p.m. UTC | #2
On 12/16/19 3:55 PM, Levente Polyak via arch-projects wrote:
> Please adjust zsh_completions.in to take multiple packages in _makerepropkg_args

I'm not a zsh user, and I don't know what this means and therefore
cannot implement it. Maybe you could do it instead, since you're the one
who has been recently authoring changes to that file?
Emil Velikov via arch-projects Dec. 19, 2019, 10:02 p.m. UTC | #3
On December 19, 2019 10:19:43 PM GMT+01:00, Eli Schwartz via arch-projects <arch-projects@archlinux.org> wrote:
>On 12/16/19 3:55 PM, Levente Polyak via arch-projects wrote:
>> Please adjust zsh_completions.in to take multiple packages in
>_makerepropkg_args
>
>I'm not a zsh user, and I don't know what this means and therefore
>cannot implement it. Maybe you could do it instead, since you're the
>one
>who has been recently authoring changes to that file?

Just replace the 1 with a asterisk * where I mentioned it. 
I'm not really into fixing up all commits people contribute to keep the integrity of completion that I once cleaned and synced to the current state.
Emil Velikov via arch-projects Dec. 19, 2019, 11:37 p.m. UTC | #4
On 12/19/19 5:02 PM, Levente Polyak via arch-projects wrote:
> Just replace the 1 with a asterisk * where I mentioned it.
> 
> I'm not really into fixing up all commits people contribute to keep
> the integrity of completion that I once cleaned and synced to the
> current state.

I'm not really into having contributions blocked contingent on me
learning zsh.

...

It's nice that you're personally motivated to update zsh completions,
but it would mean more to me if you had the same care and concern for
the rotting bash completions, which is what *I* care about, since bash
is the shell I know and use. (That, however, is not a current priority
for me, so it goes onto the endless TODO list.)

This shows me that you're asking for zsh completions because you're
enthusiastic about zsh on a personal level, not because there's a
fundamental goal that the project needs to have up to date completions. :(

In the meantime, spoonfeeding me the actual diff you want for this file
and asking me to submit it with my name slapped on as the author feels
sort of silly to me. Nevertheless, if you tell me what the range diff
should be for "Please add the new option to zsh_completions.in", I will
be happy to combine that with replacing the "1" character with a "*" as
advised above, and submit a third patch that updates the completions for
your shell of choice (but no others).

Tell me exactly what to do and I'll do it. :)

...

All I can imagine now is submitting a feature to a project and being
told "sorry, we won't accept this unless you also submit updated gettext
translations for this list of 30 languages".

This is why projects usually have dedicated stakeholders to maintain
select integration features that aren't the core project and ensure they
are kept up to date with the rest of the codebase.

And those stakeholders do their work after features are merged, and if
they don't show up to do the work, the project can still be updated and
released.
Emil Velikov via arch-projects Dec. 19, 2019, 11:49 p.m. UTC | #5
You are u justified ranting here for a whole wall of text instead of editing a simple text file that anyone can understand with zero knowledge of the specifics.
Its a part of the toolkit to the same degree as man pages are. We are also not wanting to soften up the integrity of man pages just because a contributor never touched man pages. You are more then capable of editing a single line in a text file you not fully know every detail about just by purely observing to the same degree I was able to restore its integrity without ever having done zsh completions before.
Its not the responsibility of the maintainers to fix up man pages and completions for all eternity whe also offering help and hints how to adjust them when people don't know a out them.
I restored the zsh integrity and I'm happy to point people to how to keep them in tact with their changes but I'm not a cleanup boy because someone is too lazy to change a text file.
Emil Velikov via arch-projects Dec. 20, 2019, 12:14 a.m. UTC | #6
On December 20, 2019 12:37:10 AM GMT+01:00, Eli Schwartz via arch-projects <arch-projects@archlinux.org> wrote:
>It's nice that you're personally motivated to update zsh completions,
>but it would mean more to me if you had the same care and concern for
>the rotting bash completions, which is what *I* care about, since bash
>is the shell I know and use. (That, however, is not a current priority
>for me, so it goes onto the endless TODO list.)
>
>This shows me that you're asking for zsh completions because you're
>enthusiastic about zsh on a personal level, not because there's a
>fundamental goal that the project needs to have up to date completions.
>:(
>


And to respond to this explicitly, no, this is false. I care a out the integrity
of the specific parts and not to purely suite the variant I by accident use
as well. Fixing bash completions is still on my list as well and once it's
up to date it should also be aimed to fix up on changes. The truth is as
simple as bash completions being a total mess (at least the one we have
in the repo) compared to the zsh completion we have there so within the
time frame I prioritized bringing the zsh variant back to shape was way
easier even through I have way less zsh specific knowledge compared to
having bash related knowledge.
Once bash is in tact as well I have the very exact same feelings about its
integrity as I do right now for zsh, purely to sustain integrity in the
projects components itself from a maintainer view rather than *any*
personal choice or preference, because my personal preference doesn't
matter when having the maintainer hat on.
Emil Velikov via arch-projects Dec. 20, 2019, 12:25 a.m. UTC | #7
On 12/19/19 6:49 PM, Levente Polyak via arch-projects wrote:
> You are u justified ranting here for a whole wall of text

And you are unjustified in ranting here for a single sentence about how
you're "not into fixing up the precious integrity of my precious that
you're ruining by submitting PRs". It's seriously demotivating.

I'll consider only doing my rants via pithy one-liners in the future.

> instead of editing a simple text file that anyone can understand
> with zero knowledge of the specifics.

Untrue, or are you suggesting I either understood or now currently
understand where and why to use a "1" or a "*" in your previous
recommendation "with zero knowledge of the specifics"?

How do you propose I test the changes, what makes you think I have zsh
installed at all, much less configured to initialize completions
(however that is done)?

> Its a part of the toolkit to the same degree as man pages are. We are
> also not wanting to soften up the integrity of man pages just because
> a contributor never touched man pages.

Man pages are prose, not code, so I expect that to be a lot easier. And
if someone submits a man page update with incorrect asciidoc formatting,
then 99% of the patch would still be correct and I'd be happy as a
project maintainer to fix the rest.

This is assuming that the formatting issues aren't of the variety that
causes "make all" to fail. It's not hard to update *prose* when there's
a Makefile target to prove it works, and when every user is expected to
know how to run "man".

This is quite aside for the fact that most devtools programs don't have
manpages at all, so it's a non-issue.

> Its not the responsibility of the maintainers to fix up man pages
> and completions for all eternity

It's not the job of the maintainers. It's the job of the zsh users. You
happen to be both.

I didn't say it's your responsibility to fix up bash completions for all
eternity either -- that would be my job as a concerned bash user, and
I'll take my lumps for not being on the ball for it.

> whe also offering help and hints how to adjust them when people don't
> know a out them. I restored the zsh integrity and I'm happy to point
> people to how to keep them in tact with their changes but I'm not a
> cleanup boy because someone is too lazy to change a text file.

Sure, and by the same definition of a zsh script as "just a text file"
the linux kernel is just a bunch of text files, I guess anyone who
doesn't contribute changes there is too lazy?
Emil Velikov via arch-projects Dec. 20, 2019, 12:48 a.m. UTC | #8
On December 20, 2019 1:25:57 AM GMT+01:00, Eli Schwartz via arch-projects <arch-projects@archlinux.org> wrote:
>On 12/19/19 6:49 PM, Levente Polyak via arch-projects wrote:
>> You are u justified ranting here for a whole wall of text
>
>And you are unjustified in ranting here for a single sentence about how
>you're "not into fixing up the precious integrity of my precious that
>you're ruining by submitting PRs". It's seriously demotivating.
>
>I'll consider only doing my rants via pithy one-liners in the future.
>
>> instead of editing a simple text file that anyone can understand
>> with zero knowledge of the specifics.
>
>Untrue, or are you suggesting I either understood or now currently
>understand where and why to use a "1" or a "*" in your previous
>recommendation "with zero knowledge of the specifics"?
>

Yes and that's why I helped you out with a simple answer to the question how to do it. 


>How do you propose I test the changes, what makes you think I have zsh
>installed at all, much less configured to initialize completions
>(however that is done)?
>

Seriously, look at that completion. I'm confident you have the technical competence to add a new option that simply works and it's a review here after all. 


>> Its a part of the toolkit to the same degree as man pages are. We are
>> also not wanting to soften up the integrity of man pages just because
>> a contributor never touched man pages.
>
>Man pages are prose, not code, so I expect that to be a lot easier. And
>if someone submits a man page update with incorrect asciidoc
>formatting,
>then 99% of the patch would still be correct and I'd be happy as a
>project maintainer to fix the rest.
>
>This is assuming that the formatting issues aren't of the variety that
>causes "make all" to fail. It's not hard to update *prose* when there's
>a Makefile target to prove it works, and when every user is expected to
>know how to run "man".
>
>This is quite aside for the fact that most devtools programs don't have
>manpages at all, so it's a non-issue.
>

Yep very happy you did the patches but it would be awesome of a frequent
contributor to also do the simple completion thingies. If it's something
unjustified complex that would be fine by me, but it's literally someone
so simple anyone can easily comprehend. I would prefer helping
contributors to get to a point where doing man pages and completions
is just part of a PR. 

Please for the sake of the project let's just get over this debate and get
a copy paste line for the new option into the completion file. I'm confident
you will do it properly in under 10 seconds, I know you well enough
to be able to judge that it's a zero brainer for you. 

Cheers and happy to include your series, 
anthraxx

Patch

diff --git a/doc/makerepropkg.1.asciidoc b/doc/makerepropkg.1.asciidoc
index 301b73e..0d7ddcb 100644
--- a/doc/makerepropkg.1.asciidoc
+++ b/doc/makerepropkg.1.asciidoc
@@ -7,12 +7,12 @@  makerepropkg - Rebuild a package to see if it is reproducible
 
 Synopsis
 --------
-makerepropkg [OPTIONS] <package_file>
+makerepropkg [OPTIONS] <package_file>...
 
 Description
 -----------
 
-Given the path to a built pacman package, attempt to rebuild it using the
+Given the path to a built pacman package(s), attempt to rebuild it using the
 PKGBUILD in the current directory. The package will be built in an environment
 as closely matching the environment of the initial package as possible, by
 building up a chroot to match the information exposed in the package's
@@ -20,6 +20,10 @@  linkman:BUILDINFO[5] manifest. On success, the resulting package will be
 compared to the input package, and makerepropkg will report whether the
 artifacts are identical.
 
+When given multiple packages, additional package files are assumed to be split
+packages and will be treated as additional artifacts to compare during the
+verification step.
+
 This implements a verifier for pacman/libalpm packages in accordance with the
 link:https://reproducible-builds.org/[Reproducible Builds] project.
 
diff --git a/makerepropkg.in b/makerepropkg.in
index 60fee95..51c2dd2 100755
--- a/makerepropkg.in
+++ b/makerepropkg.in
@@ -117,10 +117,13 @@  check_root
 
 if [[ -n $1 ]]; then
     pkgfile="$1"
-    if ! bsdtar -tqf "${pkgfile}" .BUILDINFO >/dev/null 2>&1; then
-        error "file is not a valid pacman package: '%s'" "${pkgfile}"
-        exit 1
-    fi
+    splitpkgs=("$@")
+    for f in "${splitpkgs[@]}"; do
+        if ! bsdtar -tqf "${f}" .BUILDINFO >/dev/null 2>&1; then
+            error "file is not a valid pacman package: '%s'" "${f}"
+            exit 1
+        fi
+    done
 else
     error "no package file specified. Try '${BASH_SOURCE[0]##*/} -h' for more information. "
     exit 1
@@ -176,23 +179,26 @@  arch-nspawn "${buildroot}/${chroot}" \
     --bind="${PWD}:/startdir" \
     --bind="${SRCDEST}:/srcdest" \
     /chrootbuild -C --noconfirm --log --holdver --skipinteg
+ret=$?
 
-if (( $? == 0 )); then
+if (( ${ret} == 0 )); then
     msg2 "built succeeded! built packages can be found in ${buildroot}/${chroot}/pkgdest"
     msg "comparing artifacts..."
 
-    comparefiles=("${pkgfile}" "${buildroot}/${chroot}/pkgdest/${pkgfile##*/}")
-    if cmp -s "${comparefiles[@]}"; then
-        msg2 "Package successfully reproduced!"
-        exit 0
-    else
-        warning "Package is not reproducible. :("
-        sha256sum "${comparefiles[@]}"
-        if (( diffoscope )); then
-            diffoscope "${comparefiles[@]}"
+    for pkgfile in "${splitpkgs[@]}"; do
+        comparefiles=("${pkgfile}" "${buildroot}/${chroot}/pkgdest/${pkgfile##*/}")
+        if cmp -s "${comparefiles[@]}"; then
+            msg2 "Package '%s' successfully reproduced!" "${pkgfile}"
+        else
+            ret=1
+            warning "Package '%s' is not reproducible. :(" "${pkgfile}"
+            sha256sum "${comparefiles[@]}"
+            if (( diffoscope )); then
+                diffoscope "${comparefiles[@]}"
+            fi
         fi
-    fi
+    done
 fi
 
-# the package either failed to build, or was unreproducible
-exit 1
+# return failure from chrootbuild, or the reproducibility status
+exit ${ret}