[PATCH 1/4] makepkg: extract parts of the write_pkginfo for use elsewhere

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/4] makepkg: extract parts of the write_pkginfo for use elsewhere

Allan McRae
From: Levente Polyak <[hidden email]>

Signed-off-by: Allan McRae <[hidden email]>
---
 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/4] makepkg: add more information to .BUILDINFO

Allan McRae
From: Levente Polyak <[hidden email]>

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 <[hidden email]>
---
 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Allan McRae
In reply to this post by Allan McRae
From: Levente Polyak <[hidden email]>

Signed-off-by: Allan McRae <[hidden email]>
---
 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 4/4] [RFC] makepkg: unify times for generated files in srcdir before packaging

Allan McRae
In reply to this post by Allan McRae
From: Levente Polyak <[hidden email]>

Signed-off-by: Allan McRae <[hidden email]>
---

[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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/4] makepkg: extract parts of the write_pkginfo for use elsewhere

dave reisner
In reply to this post by Allan McRae
On Mon, Apr 17, 2017 at 10:03:00PM +1000, Allan McRae wrote:
> From: Levente Polyak <[hidden email]>
>
> Signed-off-by: Allan McRae <[hidden email]>
> ---

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

dave reisner
In reply to this post by Allan McRae
On Mon, Apr 17, 2017 at 10:03:02PM +1000, Allan McRae wrote:

> From: Levente Polyak <[hidden email]>
>
> Signed-off-by: Allan McRae <[hidden email]>
> ---
>  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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 4/4] [RFC] makepkg: unify times for generated files in srcdir before packaging

dave reisner
In reply to this post by Allan McRae
On Mon, Apr 17, 2017 at 10:03:03PM +1000, Allan McRae wrote:

> From: Levente Polyak <[hidden email]>
>
> Signed-off-by: Allan McRae <[hidden email]>
> ---
>
> [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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Eli Schwartz
In reply to this post by Allan McRae
On 04/17/2017 08:03 AM, Allan McRae wrote:
> From: Levente Polyak <[hidden email]>
[...]>  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


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Allan McRae
In reply to this post by Allan McRae
On 21/04/17 13:36, Eli Schwartz wrote:

> On 04/20/2017 11:01 PM, Allan McRae wrote:
>> I am probably moving this to after source extraction/prepare() running,
>> so it can be skipped with --noextract.
>
> But --noextract depends on your having at some point previously run
> --nobuild, in order to pull updated sources, re-patch any patches, etc.
> which I still don't want to do manually outside of makepkg... I don't
> see why makepkg should start breaking things for me.
>
> How about instead we guard it with
> BUILDENV+=(fix_everyone_elses_SOURCE_DATE_EPOCH_stuff)
>
> Or better yet, just file bugs against whatever upstream build
> system/programming language/source code is determined to sneakily embed
> source code modification times into generated files, and call it a day?
>

Adding list back - any further off-list replies will be completely ignored.

The reproducible builds people will provide details, but it appears
pyo/pyc do this.

A
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Levente Polyak-3
On 04/21/2017 07:08 AM, Allan McRae wrote:

> On 21/04/17 13:36, Eli Schwartz wrote:
>> On 04/20/2017 11:01 PM, Allan McRae wrote:
>>> I am probably moving this to after source extraction/prepare() running,
>>> so it can be skipped with --noextract.
>>
>> But --noextract depends on your having at some point previously run
>> --nobuild, in order to pull updated sources, re-patch any patches, etc.
>> which I still don't want to do manually outside of makepkg... I don't
>> see why makepkg should start breaking things for me.
>>
>> How about instead we guard it with
>> BUILDENV+=(fix_everyone_elses_SOURCE_DATE_EPOCH_stuff)
>>
>> Or better yet, just file bugs against whatever upstream build
>> system/programming language/source code is determined to sneakily embed
>> source code modification times into generated files, and call it a day?
>>
>
> Adding list back - any further off-list replies will be completely ignored.
>
> The reproducible builds people will provide details, but it appears
> pyo/pyc do this.
>
> A
>

Unfortunately it won't be possible to fully avoid such uniform
modifications times of input files to get reproducible builds. The most
known dominator there is indeed python and its way to determine the
produced artifacts state compared to the source code. Make does use the
same information by setting the modification time of the produced
artifacts to compare them.
I agree make is in that detail a nicer way as that info in contained
outside of the content of the produced artifacts itself, however there
are external design decisions that we need to accept that they exist and
won't change.

So as both use cases (reproducible and incremental builds) seem to be
valid and wanted features, let's be cooperative and see how we can make
both separately work to make all of us happy. :)

As I don't really see how both can work at the same time: one way that
could do its job is an option like --incremental that would not do any
timestamp unification. This could be used in the case of a incremental
build of a VCS package in a non cleaned environment. I think that is not
too much of a hassle to do in case someone wants to build an VCS package
incrementally with previous object files?

cheers,
Levente
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Andrew Gregory
On 05/02/17 at 09:30pm, Levente Polyak wrote:

> On 04/21/2017 07:08 AM, Allan McRae wrote:
> > On 21/04/17 13:36, Eli Schwartz wrote:
> >> On 04/20/2017 11:01 PM, Allan McRae wrote:
> >>> I am probably moving this to after source extraction/prepare() running,
> >>> so it can be skipped with --noextract.
> >>
> >> But --noextract depends on your having at some point previously run
> >> --nobuild, in order to pull updated sources, re-patch any patches, etc.
> >> which I still don't want to do manually outside of makepkg... I don't
> >> see why makepkg should start breaking things for me.
> >>
> >> How about instead we guard it with
> >> BUILDENV+=(fix_everyone_elses_SOURCE_DATE_EPOCH_stuff)
> >>
> >> Or better yet, just file bugs against whatever upstream build
> >> system/programming language/source code is determined to sneakily embed
> >> source code modification times into generated files, and call it a day?
> >>
> >
> > Adding list back - any further off-list replies will be completely ignored.
> >
> > The reproducible builds people will provide details, but it appears
> > pyo/pyc do this.
> >
> > A
>
> Unfortunately it won't be possible to fully avoid such uniform
> modifications times of input files to get reproducible builds. The most
> known dominator there is indeed python and its way to determine the
> produced artifacts state compared to the source code. Make does use the
> same information by setting the modification time of the produced
> artifacts to compare them.
> I agree make is in that detail a nicer way as that info in contained
> outside of the content of the produced artifacts itself, however there
> are external design decisions that we need to accept that they exist and
> won't change.
>
> So as both use cases (reproducible and incremental builds) seem to be
> valid and wanted features, let's be cooperative and see how we can make
> both separately work to make all of us happy. :)
>
> As I don't really see how both can work at the same time: one way that
> could do its job is an option like --incremental that would not do any
> timestamp unification. This could be used in the case of a incremental
> build of a VCS package in a non cleaned environment. I think that is not
> too much of a hassle to do in case someone wants to build an VCS package
> incrementally with previous object files?

Why is setting the modification timestamp necessary?  makepkg should
be preserving the modification timestamps of files when it extracts
them from archives.  So two builds using the same source tarballs
should already have files with the same timestamps.  When is this not
sufficient?

apg
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

Levente Polyak-3
On 05/02/2017 10:09 PM, Andrew Gregory wrote:

> On 05/02/17 at 09:30pm, Levente Polyak wrote:
>> On 04/21/2017 07:08 AM, Allan McRae wrote:
>>> On 21/04/17 13:36, Eli Schwartz wrote:
>>>> On 04/20/2017 11:01 PM, Allan McRae wrote:
>>>>> I am probably moving this to after source extraction/prepare() running,
>>>>> so it can be skipped with --noextract.
>>>>
>>>> But --noextract depends on your having at some point previously run
>>>> --nobuild, in order to pull updated sources, re-patch any patches, etc.
>>>> which I still don't want to do manually outside of makepkg... I don't
>>>> see why makepkg should start breaking things for me.
>>>>
>>>> How about instead we guard it with
>>>> BUILDENV+=(fix_everyone_elses_SOURCE_DATE_EPOCH_stuff)
>>>>
>>>> Or better yet, just file bugs against whatever upstream build
>>>> system/programming language/source code is determined to sneakily embed
>>>> source code modification times into generated files, and call it a day?
>>>>
>>>
>>> Adding list back - any further off-list replies will be completely ignored.
>>>
>>> The reproducible builds people will provide details, but it appears
>>> pyo/pyc do this.
>>>
>>> A
>>
>> Unfortunately it won't be possible to fully avoid such uniform
>> modifications times of input files to get reproducible builds. The most
>> known dominator there is indeed python and its way to determine the
>> produced artifacts state compared to the source code. Make does use the
>> same information by setting the modification time of the produced
>> artifacts to compare them.
>> I agree make is in that detail a nicer way as that info in contained
>> outside of the content of the produced artifacts itself, however there
>> are external design decisions that we need to accept that they exist and
>> won't change.
>>
>> So as both use cases (reproducible and incremental builds) seem to be
>> valid and wanted features, let's be cooperative and see how we can make
>> both separately work to make all of us happy. :)
>>
>> As I don't really see how both can work at the same time: one way that
>> could do its job is an option like --incremental that would not do any
>> timestamp unification. This could be used in the case of a incremental
>> build of a VCS package in a non cleaned environment. I think that is not
>> too much of a hassle to do in case someone wants to build an VCS package
>> incrementally with previous object files?
>
> Why is setting the modification timestamp necessary?  makepkg should
> be preserving the modification timestamps of files when it extracts
> them from archives.  So two builds using the same source tarballs
> should already have files with the same timestamps.  When is this not
> sufficient?
>
> apg
>


Reproducibility is not something that should be exclusive to archives
providing such anyway. There is an increasing interest in builds from
repository checkout. On top, prepare() modifies such so those should be
adjusted too before the build starts.

cheers,
Levente
Loading...