[PATCH 0/2] Add Architecture information to .BUILDINFO

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Add Architecture information to .BUILDINFO

Robin Broda
For reproducible builds, Foxboron and I are working on scripts to
reproduce a given package file. Generating archive links to download
the same exact packages that were present on the build machine is
a crucial part of this.

The package names have architecture information appended to them, eg.
archlinux-keyring-20180302-1-any instead of archlinux-keyring-20180302-1
so to be able to reliably regenerate the package filename, architecture
information is required.

The proposed patches add
- An additional field, 'arch', to the .BUILDINFO file
- The '-$arch' suffix used in package filenames
    to the packages in the 'installed' array of the .BUILDINFO

Robin Broda (2):
  Add 'arch' field to .BUILDINFO
  Append '-$arch' to 'installed' array in .BUILDINFO

 doc/BUILDINFO.5.txt   |  5 ++++-
 scripts/makepkg.sh.in | 13 +++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

--
2.16.2
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Add 'arch' field to .BUILDINFO

Robin Broda
Signed-off-by: Robin Broda <[hidden email]>
---
 doc/BUILDINFO.5.txt   | 3 +++
 scripts/makepkg.sh.in | 1 +
 2 files changed, 4 insertions(+)

diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
index b7a72831..4734301e 100644
--- a/doc/BUILDINFO.5.txt
+++ b/doc/BUILDINFO.5.txt
@@ -44,6 +44,9 @@ BUILDINFO file format.
 *packager*::
  The details of the packager that built the package.
 
+*arch*::
+ The architecture of the package.
+
 *builddate*::
  The build date of the package in epoch.
 
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 63b6c3e1..ece53dca 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -688,6 +688,7 @@ write_buildinfo() {
  write_kv_pair "pkgbuild_sha256sum" $sum
 
  write_kv_pair "packager" "${PACKAGER}"
+ write_kv_pair "arch" "$pkgarch"
  write_kv_pair "builddate" "${SOURCE_DATE_EPOCH}"
  write_kv_pair "builddir"  "${BUILDDIR}"
  write_kv_pair "buildenv" "${BUILDENV[@]}"
--
2.16.2
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Append '-$arch' to 'installed' array in .BUILDINFO

Robin Broda
In reply to this post by Robin Broda
This patch incurs a **severe** performance degradation when generating
the .BUILDINFO file, likely due to frequent usage of `pacman -Qi`
and `grep -E`. I haven't found a faster way to gather this information.

Signed-off-by: Robin Broda <[hidden email]>
---
 doc/BUILDINFO.5.txt   |  2 +-
 scripts/makepkg.sh.in | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
index 4734301e..2c74f9ff 100644
--- a/doc/BUILDINFO.5.txt
+++ b/doc/BUILDINFO.5.txt
@@ -61,7 +61,7 @@ BUILDINFO file format.
 
 *installed (array)*::
  The installed packages at build time including the version information of
- the package. Formatted as "$pkgname-$pkgver-$pkgrel".
+ the package. Formatted as "$pkgname-$pkgver-$pkgrel-$pkgarch".
 
 See Also
 --------
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index ece53dca..10303417 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -694,8 +694,16 @@ write_buildinfo() {
  write_kv_pair "buildenv" "${BUILDENV[@]}"
  write_kv_pair "options" "${OPTIONS[@]}"
 
- local pkglist=($(run_pacman -Q | sed "s# #-#"))
- write_kv_pair "installed" "${pkglist[@]}"
+ local pkglist=($(run_pacman -Qq))
+ local installed=()
+ for pkg in "${pkglist[@]}"
+ do
+ pkginfo="$(pacman -Qi "${pkg}")"
+ pkgver="$(grep -E '^Version' <<< "${pkginfo}" | tr -d ' ' | cut -d':' -f2-)"
+ pkgarch="$(grep -E '^Architecture' <<< "${pkginfo}" | tr -d ' ' | cut -d':' -f2-)"
+ installed+=("${pkg}-${pkgver}-${pkgarch}")
+ done
+ write_kv_pair "installed" "${installed[@]}"
 }
 
 # build a sorted NUL-separated list of the full contents of the current
--
2.16.2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Append '-$arch' to 'installed' array in .BUILDINFO

Eli Schwartz-2
On 03/17/2018 05:24 PM, Robin Broda wrote:
> This patch incurs a **severe** performance degradation when generating
> the .BUILDINFO file, likely due to frequent usage of `pacman -Qi`
> and `grep -E`. I haven't found a faster way to gather this information.

I'm not sure we can, without adding a dependency on e.g. expac. Which
would admittedly be a lot cleaner and we could also remove the existing
sed command.

The downside is depending on a pretty specific tool. The plus side is
ensuring that everyone has a pretty cool tool installed by default...

pacman itself does not really give output meant for parsing.

...

The alternative, reproduction-wise, is of course to brute force by
trying to access both "x86_64" and "any" (it would suffice to check the
return code of curl -IfLls -o /dev/null $url). I don't think this is a
terrible option, unless Allan is willing to accept expac-based patches...

--
Eli Schwartz
Bug Wrangler and Trusted User


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

Re: [PATCH 1/2] Add 'arch' field to .BUILDINFO

Eli Schwartz-2
In reply to this post by Robin Broda
On 03/17/2018 05:24 PM, Robin Broda wrote:

> Signed-off-by: Robin Broda <[hidden email]>
> ---
>  doc/BUILDINFO.5.txt   | 3 +++
>  scripts/makepkg.sh.in | 1 +
>  2 files changed, 4 insertions(+)
>
> diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
> index b7a72831..4734301e 100644
> --- a/doc/BUILDINFO.5.txt
> +++ b/doc/BUILDINFO.5.txt
> @@ -44,6 +44,9 @@ BUILDINFO file format.
>  *packager*::
>   The details of the packager that built the package.
>  
> +*arch*::
> + The architecture of the package.
> +
I question the utility of duplicating this information in both
.BUILDINFO and .PKGINFO

--
Eli Schwartz
Bug Wrangler and Trusted User


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

Re: [PATCH 1/2] Add 'arch' field to .BUILDINFO

Allan McRae
On 18/03/18 11:22, Eli Schwartz wrote:

> On 03/17/2018 05:24 PM, Robin Broda wrote:
>> Signed-off-by: Robin Broda <[hidden email]>
>> ---
>>  doc/BUILDINFO.5.txt   | 3 +++
>>  scripts/makepkg.sh.in | 1 +
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
>> index b7a72831..4734301e 100644
>> --- a/doc/BUILDINFO.5.txt
>> +++ b/doc/BUILDINFO.5.txt
>> @@ -44,6 +44,9 @@ BUILDINFO file format.
>>  *packager*::
>>   The details of the packager that built the package.
>>  
>> +*arch*::
>> + The architecture of the package.
>> +
>
> I question the utility of duplicating this information in both
> .BUILDINFO and .PKGINFO

There is already a bunch duplicated across those files.  The goal was to
have everything needed to build the package in a reproducible way in the
.BUILDINFO file.

I do question why it was output at that point, rather than under the
version info where it seems much more natural.

A
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Append '-$arch' to 'installed' array in .BUILDINFO

Allan McRae
In reply to this post by Robin Broda
On 18/03/18 07:24, Robin Broda wrote:
> This patch incurs a **severe** performance degradation when generating
> the .BUILDINFO file, likely due to frequent usage of `pacman -Qi`
> and `grep -E`. I haven't found a faster way to gather this information.
>
> Signed-off-by: Robin Broda <[hidden email]>

I will comment on the utility of this patch in another email.   This is
just pointing out that it is broken...

> ---
>  doc/BUILDINFO.5.txt   |  2 +-
>  scripts/makepkg.sh.in | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
> index 4734301e..2c74f9ff 100644
> --- a/doc/BUILDINFO.5.txt
> +++ b/doc/BUILDINFO.5.txt
> @@ -61,7 +61,7 @@ BUILDINFO file format.
>  
>  *installed (array)*::
>   The installed packages at build time including the version information of
> - the package. Formatted as "$pkgname-$pkgver-$pkgrel".
> + the package. Formatted as "$pkgname-$pkgver-$pkgrel-$pkgarch".
>  
>  See Also
>  --------
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index ece53dca..10303417 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -694,8 +694,16 @@ write_buildinfo() {
>   write_kv_pair "buildenv" "${BUILDENV[@]}"
>   write_kv_pair "options" "${OPTIONS[@]}"
>  
> - local pkglist=($(run_pacman -Q | sed "s# #-#"))
> - write_kv_pair "installed" "${pkglist[@]}"
> + local pkglist=($(run_pacman -Qq))
> + local installed=()
> + for pkg in "${pkglist[@]}"
> + do
> + pkginfo="$(pacman -Qi "${pkg}")"

What makes this call to pacman not need to use run_pacman like the others?

> + pkgver="$(grep -E '^Version' <<< "${pkginfo}" | tr -d ' ' | cut -d':' -f2-)"
> + pkgarch="$(grep -E '^Architecture' <<< "${pkginfo}" | tr -d ' ' | cut -d':' -f2-)"

Not every system runs in English...

> + installed+=("${pkg}-${pkgver}-${pkgarch}")
> + done
> + write_kv_pair "installed" "${installed[@]}"
>  }
>  
>  # build a sorted NUL-separated list of the full contents of the current
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Add Architecture information to .BUILDINFO

Allan McRae
In reply to this post by Robin Broda
On 18/03/18 07:23, Robin Broda wrote:
> For reproducible builds, Foxboron and I are working on scripts to
> reproduce a given package file. Generating archive links to download
> the same exact packages that were present on the build machine is
> a crucial part of this.
>
> The package names have architecture information appended to them, eg.
> archlinux-keyring-20180302-1-any instead of archlinux-keyring-20180302-1
> so to be able to reliably regenerate the package filename, architecture
> information is required.

Isn't this being handled elsewhere already - e.g. On the Debian provided
reproducible service?  How is that being managed without the
architecture information?

A
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Add Architecture information to .BUILDINFO

Morten Linderud-2
On Sun, Mar 18, 2018 at 11:52:01AM +1000, Allan McRae wrote:

> On 18/03/18 07:23, Robin Broda wrote:
> > For reproducible builds, Foxboron and I are working on scripts to
> > reproduce a given package file. Generating archive links to download
> > the same exact packages that were present on the build machine is
> > a crucial part of this.
> >
> > The package names have architecture information appended to them, eg.
> > archlinux-keyring-20180302-1-any instead of archlinux-keyring-20180302-1
> > so to be able to reliably regenerate the package filename, architecture
> > information is required.
>
> Isn't this being handled elsewhere already - e.g. On the Debian provided
> reproducible service?  How is that being managed without the
> architecture information?
>
> A
Their service doesn't care as it tracks SVN directly when building packages.
However, when we provide repro tools for our users we need to recreate with
`.BUILDINFO`. So we have to pull down packages from the archive. Jelle suggested
we just bruteforce it for the time being. But that sucks a little bit.

Tool in question: https://github.com/Foxboron/devtools-repro 

--
Morten Linderud

PGP: 9C02FF419FECBE16

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

Re: [PATCH 0/2] Add Architecture information to .BUILDINFO

Andrew Gregory
On 03/18/18 at 02:56am, Morten Linderud wrote:

> On Sun, Mar 18, 2018 at 11:52:01AM +1000, Allan McRae wrote:
> > On 18/03/18 07:23, Robin Broda wrote:
> > > For reproducible builds, Foxboron and I are working on scripts to
> > > reproduce a given package file. Generating archive links to download
> > > the same exact packages that were present on the build machine is
> > > a crucial part of this.
> > >
> > > The package names have architecture information appended to them, eg.
> > > archlinux-keyring-20180302-1-any instead of archlinux-keyring-20180302-1
> > > so to be able to reliably regenerate the package filename, architecture
> > > information is required.
> >
> > Isn't this being handled elsewhere already - e.g. On the Debian provided
> > reproducible service?  How is that being managed without the
> > architecture information?
> >
> > A
>
> Their service doesn't care as it tracks SVN directly when building packages.
> However, when we provide repro tools for our users we need to recreate with
> `.BUILDINFO`. So we have to pull down packages from the archive. Jelle suggested
> we just bruteforce it for the time being. But that sucks a little bit.
>
> Tool in question: https://github.com/Foxboron/devtools-repro 

alpm does not limit the package architecture; it can contain '-'.  If
the user has such a package installed, this will result in entries
that will be parsed incorrectly.