[pacman-dev] [PATCH 1/4] makepkg: extract parts of the write_pkginfo for use elsewhere
From: Levente Polyak <anthraxx@archlinux.org> Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 42a76004..d61c7fff 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -608,6 +608,15 @@ find_libprovides() { (( ${#libprovides[@]} )) && printf '%s\n' "${libprovides[@]}" } +get_packager() { + if [[ -n $PACKAGER ]]; then + local packager="$PACKAGER" + else + local packager="Unknown Packager" + fi + printf "%s\n" "$packager" +} + write_kv_pair() { local key="$1" shift @@ -621,13 +630,22 @@ write_kv_pair() { done } -write_pkginfo() { - if [[ -n $PACKAGER ]]; then - local packager="$PACKAGER" - else - local packager="Unknown Packager" +write_kv_pkgname() { + write_kv_pair "pkgname" "$pkgname" + if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then + write_kv_pair "pkgbase" "$pkgbase" + fi +} + +write_kv_pkgver() { + local fullver=$(get_full_version) + write_kv_pair "pkgver" "$fullver" + if [[ "$fullver" != "$basever" ]]; then + write_kv_pair "basever" "$basever" fi +} +write_pkginfo() { local size="$(@DUPATH@ @DUFLAGS@)" size="$(( ${size%%[^0-9]*} * 1024 ))" @@ -637,16 +655,8 @@ write_pkginfo() { printf "# Generated by makepkg %s\n" "$makepkg_version" printf "# using %s\n" "$(fakeroot -v)" - write_kv_pair "pkgname" "$pkgname" - if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then - write_kv_pair "pkgbase" "$pkgbase" - fi - - local fullver=$(get_full_version) - write_kv_pair "pkgver" "$fullver" - if [[ "$fullver" != "$basever" ]]; then - write_kv_pair "basever" "$basever" - fi + write_kv_pkgname + write_kv_pkgver # TODO: all fields should have this treatment local spd="${pkgdesc//+([[:space:]])/ }" @@ -656,7 +666,7 @@ write_pkginfo() { write_kv_pair "pkgdesc" "$spd" write_kv_pair "url" "$url" write_kv_pair "builddate" "$SOURCE_DATE_EPOCH" - write_kv_pair "packager" "$packager" + write_kv_pair "packager" "$(get_packager)" write_kv_pair "size" "$size" write_kv_pair "arch" "$pkgarch" -- 2.12.0
From: Levente Polyak <anthraxx@archlinux.org> The .BUILDINFO file should retain all the information needed to reproducibly build a package. Add some extra information to the file and also provide a version number to keep track of future changes. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d61c7fff..7692ade5 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -688,13 +688,17 @@ write_pkginfo() { write_buildinfo() { msg2 "$(gettext "Generating %s file...")" ".BUILDINFO" - write_kv_pair "builddir" "${BUILDDIR}" + write_kv_pair "format" "1" + write_kv_pkgname + write_kv_pkgver local sum="$(sha256sum "${BUILDFILE}")" sum=${sum%% *} - write_kv_pair "pkgbuild_sha256sum" $sum + write_kv_pair "packager" "$(get_packager)" + write_kv_pair "builddate" "${SOURCE_DATE_EPOCH}" + write_kv_pair "builddir" "${BUILDDIR}" write_kv_pair "buildenv" "${BUILDENV[@]}" write_kv_pair "options" "${OPTIONS[@]}" -- 2.12.0
From: Levente Polyak <anthraxx@archlinux.org> Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 7692ade5..df4d6a06 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -475,6 +475,9 @@ run_prepare() { } run_build() { + # unify source times before building for reproducibility + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \; + run_function_safe "build" } -- 2.12.0
On Mon, Apr 17, 2017 at 10:03:02PM +1000, Allan McRae wrote:
From: Levente Polyak <anthraxx@archlinux.org>
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 7692ade5..df4d6a06 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -475,6 +475,9 @@ run_prepare() { }
run_build() { + # unify source times before building for reproducibility + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \; +
I'd use the '{} +' form of find here to avoid excessive forking.
run_function_safe "build" }
-- 2.12.0
On 04/17/2017 08:03 AM, Allan McRae wrote:
From: Levente Polyak <anthraxx@archlinux.org> [...]> run_build() { + # unify source times before building for reproducibility + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \; + run_function_safe "build" }
Just a general question on "why do we want this" (and the followup patch to do this at the beginning of the package() step)... It is one thing for makepkg to fiddle with its own internal logic to respect SOURCE_DATE_EPOCH with regard to package metadata, installed file modification times, etc. but as mentioned in the other thread, it is not makepkg's job to ensure that, for example, python's compiled bytecode respects SOURCE_DATE_EPOCH. Any build/generation process that changes its own output based on the reported date of the source files, is doing something wrong anyway. Moreover, this breaks incremental builds by making the build system think all files have been modified and must be recompiled. Incremental builds are currently a perfectly valid use case for e.g. *-git or other devel packages (assuming one is building for their own computer, isn't worried about automagic/non-clean-chroot dependencies, and is reasonably confident that the build system in question doesn't fall on its face when asked to do incremental builds). I very much do not want this to be accepted. :) -- Eli Schwartz
From: Levente Polyak <anthraxx@archlinux.org> Signed-off-by: Allan McRae <allan@archlinux.org> --- [Allan] I'm told his is useful for some python packages that generate pyo/pyc files during package... I am undecided about its suitability for inclusion in makepkg yet. scripts/makepkg.sh.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index df4d6a06..84b83e7d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -493,6 +493,8 @@ run_package() { pkgfunc="package_$1" fi + # unify source times before package for reproducibility + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \; run_function_safe "$pkgfunc" } -- 2.12.0
On Mon, Apr 17, 2017 at 10:03:03PM +1000, Allan McRae wrote:
From: Levente Polyak <anthraxx@archlinux.org>
Signed-off-by: Allan McRae <allan@archlinux.org> ---
[Allan] I'm told his is useful for some python packages that generate pyo/pyc files during package... I am undecided about its suitability for inclusion in makepkg yet.
scripts/makepkg.sh.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index df4d6a06..84b83e7d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -493,6 +493,8 @@ run_package() { pkgfunc="package_$1" fi
+ # unify source times before package for reproducibility + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \;
Same as 3/4 -- prefer {} +. If we accept this patch, the commit message should include an explanation as to why this is useful.
run_function_safe "$pkgfunc" }
-- 2.12.0
On Mon, Apr 17, 2017 at 10:03:00PM +1000, Allan McRae wrote:
From: Levente Polyak <anthraxx@archlinux.org>
Signed-off-by: Allan McRae <allan@archlinux.org> ---
Sorry, a lot of these comments are irrelevant to the actual patch, but I couldn't help pointing them out...
scripts/makepkg.sh.in | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 42a76004..d61c7fff 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -608,6 +608,15 @@ find_libprovides() { (( ${#libprovides[@]} )) && printf '%s\n' "${libprovides[@]}" }
+get_packager() { + if [[ -n $PACKAGER ]]; then + local packager="$PACKAGER" + else + local packager="Unknown Packager" + fi + printf "%s\n" "$packager"
I was going to suggest that we simply make this: printf '%s\n' "${PACKAGER:-Unknown Packager}" But then it occurred to me that if we just set this default value up front, we don't need to treat this var as special... Actually relevant to this patch, why not define this as 'write_kv_packager' to match other functions here, like 'write_kv_pkgname' and 'write_kv_pkgver'?
+} + write_kv_pair() { local key="$1" shift @@ -621,13 +630,22 @@ write_kv_pair() { done }
-write_pkginfo() { - if [[ -n $PACKAGER ]]; then - local packager="$PACKAGER" - else - local packager="Unknown Packager" +write_kv_pkgname() { + write_kv_pair "pkgname" "$pkgname" + if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then + write_kv_pair "pkgbase" "$pkgbase" + fi
Wouldn't it be nice if we just *always* wrote the pkgbase?
+} + +write_kv_pkgver() { + local fullver=$(get_full_version) + write_kv_pair "pkgver" "$fullver" + if [[ "$fullver" != "$basever" ]]; then + write_kv_pair "basever" "$basever" fi
Since 8a02abcf19, disallow pkgver overrides in package functions. Therefore, I'm unclear on when we'd ever emit this basever attr.
+}
+write_pkginfo() { local size="$(@DUPATH@ @DUFLAGS@)" size="$(( ${size%%[^0-9]*} * 1024 ))"
@@ -637,16 +655,8 @@ write_pkginfo() { printf "# Generated by makepkg %s\n" "$makepkg_version" printf "# using %s\n" "$(fakeroot -v)"
- write_kv_pair "pkgname" "$pkgname" - if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then - write_kv_pair "pkgbase" "$pkgbase" - fi - - local fullver=$(get_full_version) - write_kv_pair "pkgver" "$fullver" - if [[ "$fullver" != "$basever" ]]; then - write_kv_pair "basever" "$basever" - fi + write_kv_pkgname + write_kv_pkgver
# TODO: all fields should have this treatment local spd="${pkgdesc//+([[:space:]])/ }" @@ -656,7 +666,7 @@ write_pkginfo() { write_kv_pair "pkgdesc" "$spd" write_kv_pair "url" "$url" write_kv_pair "builddate" "$SOURCE_DATE_EPOCH" - write_kv_pair "packager" "$packager" + write_kv_pair "packager" "$(get_packager)" write_kv_pair "size" "$size" write_kv_pair "arch" "$pkgarch"
-- 2.12.0
participants (3)
-
Allan McRae
-
Dave Reisner
-
Eli Schwartz