[arch-projects] [devtools] [PATCH 0/3] makerepropkg improvements
Available at https://github.com/eli-schwartz/devtools/commits/makerepropkg-diffoscope Eli Schwartz (3): makerepropkg: fix wonky indent makerepropkg: add support to check unreproducible packages using diffoscope makerepropkg: support checking multiple split packages doc/makerepropkg.1.asciidoc | 11 ++++++-- makerepropkg.in | 56 +++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 23 deletions(-) -- 2.24.1
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- makerepropkg.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/makerepropkg.in b/makerepropkg.in index 710f3ca..2b0945e 100755 --- a/makerepropkg.in +++ b/makerepropkg.in @@ -101,12 +101,12 @@ __EOF__ } while getopts 'M:c:h' arg; do - case "$arg" in - M) archroot_args+=(-M "$OPTARG") ;; - c) cache_dirs+=("$OPTARG") ;; - h) usage; exit 0 ;; - *|?) usage; exit 1 ;; - esac + case "$arg" in + M) archroot_args+=(-M "$OPTARG") ;; + c) cache_dirs+=("$OPTARG") ;; + h) usage; exit 0 ;; + *|?) usage; exit 1 ;; + esac done shift $((OPTIND - 1)) -- 2.24.1
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- doc/makerepropkg.1.asciidoc | 3 +++ makerepropkg.in | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/doc/makerepropkg.1.asciidoc b/doc/makerepropkg.1.asciidoc index 7d68e5e..301b73e 100644 --- a/doc/makerepropkg.1.asciidoc +++ b/doc/makerepropkg.1.asciidoc @@ -26,6 +26,9 @@ link:https://reproducible-builds.org/[Reproducible Builds] project. Options ------- +*-d*:: + If packages are not reproducible, compare them using diffoscope. + *-c*:: Set the pacman cache directory. diff --git a/makerepropkg.in b/makerepropkg.in index 2b0945e..60fee95 100755 --- a/makerepropkg.in +++ b/makerepropkg.in @@ -29,6 +29,7 @@ declare -a buildenv buildopts installed installpkgs archiveurl='https://archive.archlinux.org/packages' buildroot=/var/lib/archbuild/reproducible chroot=testenv +diffoscope=0 parse_buildinfo() { local line var val @@ -94,14 +95,16 @@ package, including the .PKGINFO as well as the buildinfo. For more details see https://reproducible-builds.org/ OPTIONS + -d Run diffoscope if the package is unreproducible -c <dir> Set pacman cache -M <file> Location of a makepkg config file -h Show this usage message __EOF__ } -while getopts 'M:c:h' arg; do +while getopts 'dM:c:h' arg; do case "$arg" in + d) diffoscope=1 ;; M) archroot_args+=(-M "$OPTARG") ;; c) cache_dirs+=("$OPTARG") ;; h) usage; exit 0 ;; @@ -177,12 +180,17 @@ arch-nspawn "${buildroot}/${chroot}" \ if (( $? == 0 )); then msg2 "built succeeded! built packages can be found in ${buildroot}/${chroot}/pkgdest" msg "comparing artifacts..." - if cmp -s "${pkgfile}" "${buildroot}/${chroot}/pkgdest/${pkgfile##*/}"; then + + comparefiles=("${pkgfile}" "${buildroot}/${chroot}/pkgdest/${pkgfile##*/}") + if cmp -s "${comparefiles[@]}"; then msg2 "Package successfully reproduced!" exit 0 else warning "Package is not reproducible. :(" - sha256sum "${pkgfile}" "${buildroot}/${chroot}/pkgdest/${pkgfile##*/}" + sha256sum "${comparefiles[@]}" + if (( diffoscope )); then + diffoscope "${comparefiles[@]}" + fi fi fi -- 2.24.1
Please add the new option to zsh_completions.in
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(-) 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} -- 2.24.1
Please adjust zsh_completions.in to take multiple packages in _makerepropkg_args
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? -- Eli Schwartz Bug Wrangler and Trusted User
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.
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. -- Eli Schwartz Bug Wrangler and Trusted User
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.
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? -- Eli Schwartz Bug Wrangler and Trusted User
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
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.
Thanks to anthraxx for the guidance. Original-patch-by: Levente Polyak <anthraxx@archlinux.org> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- zsh_completion.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zsh_completion.in b/zsh_completion.in index a1225aa..e59bc3a 100644 --- a/zsh_completion.in +++ b/zsh_completion.in @@ -94,10 +94,11 @@ _offload_build_args=( ) _makerepropkg_args=( + '-d[Run diffoscope if the package is unreproducible]' '-c[Set pacman cache]:pacman_cache:_files -/' '-M[Location of a makepkg config file]:makepkg_config:_files -g "*.conf(.)"' '-h[Display usage]' - '1:working_dir:_files -g "*.pkg.tar.*(.)"' + '*:working_dir:_files -g "*.pkg.tar.*(.)"' ) _devtools_completions_all_packages() { -- 2.24.1
participants (2)
-
Eli Schwartz
-
Levente Polyak