native/cross: Add ar wrapper for determinism

Message ID 20211123141210.28396-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit 59922c95fcb20c66634c5677012d490be2246b0b
Headers show
Series native/cross: Add ar wrapper for determinism | expand

Commit Message

Richard Purdie Nov. 23, 2021, 2:12 p.m. UTC
From: Jacob Kroon <jacob.kroon@gmail.com>

Add a wrapper around ar calls for native/cross recipes. This wrapper adds
the -D option so that deterministic archives are built for native/cross
output. This improves the changes of hash equivalence matches and hence
build artefact reuse.

We don't need this in the target case since we compile binutils-cross
with an option making this the default. We need a wrapper since we need
to remove the "u" option and replace it with "D" but also allow things like
"--version" to continue to work too.

Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/cross.bbclass  |  2 ++
 scripts/cross-intercept/ar  |  1 +
 scripts/native-intercept/ar | 29 +++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)
 create mode 120000 scripts/cross-intercept/ar
 create mode 100755 scripts/native-intercept/ar

Comments

Jacob Kroon Nov. 23, 2021, 8:22 p.m. UTC | #1
On 11/23/21 15:12, Richard Purdie wrote:
> From: Jacob Kroon <jacob.kroon@gmail.com>
> 
> Add a wrapper around ar calls for native/cross recipes. This wrapper adds
> the -D option so that deterministic archives are built for native/cross
> output. This improves the changes of hash equivalence matches and hence
> build artefact reuse.
> 
> We don't need this in the target case since we compile binutils-cross
> with an option making this the default. We need a wrapper since we need
> to remove the "u" option and replace it with "D" but also allow things like
> "--version" to continue to work too.
> 
> Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/cross.bbclass  |  2 ++
>  scripts/cross-intercept/ar  |  1 +
>  scripts/native-intercept/ar | 29 +++++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+)
>  create mode 120000 scripts/cross-intercept/ar
>  create mode 100755 scripts/native-intercept/ar
> 
> diff --git a/meta/classes/cross.bbclass b/meta/classes/cross.bbclass
> index 3e6a2f60b9e..9d951076a78 100644
> --- a/meta/classes/cross.bbclass
> +++ b/meta/classes/cross.bbclass
> @@ -93,3 +93,5 @@ python do_addto_recipe_sysroot () {
>  }
>  addtask addto_recipe_sysroot after do_populate_sysroot
>  do_addto_recipe_sysroot[deptask] = "do_populate_sysroot"
> +
> +PATH:prepend = "${COREBASE}/scripts/cross-intercept:"
> diff --git a/scripts/cross-intercept/ar b/scripts/cross-intercept/ar
> new file mode 120000
> index 00000000000..bc68ffd7a21
> --- /dev/null
> +++ b/scripts/cross-intercept/ar
> @@ -0,0 +1 @@
> +../native-intercept/ar
> \ No newline at end of file
> diff --git a/scripts/native-intercept/ar b/scripts/native-intercept/ar
> new file mode 100755
> index 00000000000..642b6eae864
> --- /dev/null
> +++ b/scripts/native-intercept/ar
> @@ -0,0 +1,29 @@
> +#!/usr/bin/env python3
> +#
> +# Wrapper around 'ar' that defaults to deterministic archives
> +
> +import os
> +import shutil
> +import sys
> +
> +# calculate path to the real 'ar'
> +path = os.environ['PATH']
> +path = path.replace(os.path.dirname(sys.argv[0]), '')
> +real_ar = shutil.which('ar', path=path)
> +
> +if len(sys.argv) == 1:
> +    os.execl(real_ar, 'ar')
> +
> +# modify args to mimic 'ar' configured with --default-deterministic-archives
> +argv = sys.argv
> +if argv[1].startswith('--'):
> +    # No modifier given
> +    None
> +else:
> +    # remove the optional '-'
> +    if argv[1][0] == '-':
> +        argv[1] = argv[1][1:]
> +    argv[1] = argv[1].replace('u', '')
> +    argv[1] = 'D' + argv[1]
> +
> +os.execv(real_ar, argv)
> 

What should we do if the user is explicitly passing U, "Do not operate
in deterministic mode." ? Don't replace any 'u' and don't pass D ?

Jacob
Richard Purdie Nov. 23, 2021, 11:06 p.m. UTC | #2
On Tue, 2021-11-23 at 21:22 +0100, Jacob Kroon wrote:
> 
> On 11/23/21 15:12, Richard Purdie wrote:
> > From: Jacob Kroon <jacob.kroon@gmail.com>
> > 
> > Add a wrapper around ar calls for native/cross recipes. This wrapper adds
> > the -D option so that deterministic archives are built for native/cross
> > output. This improves the changes of hash equivalence matches and hence
> > build artefact reuse.
> > 
> > We don't need this in the target case since we compile binutils-cross
> > with an option making this the default. We need a wrapper since we need
> > to remove the "u" option and replace it with "D" but also allow things like
> > "--version" to continue to work too.
> > 
> > Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  meta/classes/cross.bbclass  |  2 ++
> >  scripts/cross-intercept/ar  |  1 +
> >  scripts/native-intercept/ar | 29 +++++++++++++++++++++++++++++
> >  3 files changed, 32 insertions(+)
> >  create mode 120000 scripts/cross-intercept/ar
> >  create mode 100755 scripts/native-intercept/ar
> > 
> > diff --git a/meta/classes/cross.bbclass b/meta/classes/cross.bbclass
> > index 3e6a2f60b9e..9d951076a78 100644
> > --- a/meta/classes/cross.bbclass
> > +++ b/meta/classes/cross.bbclass
> > @@ -93,3 +93,5 @@ python do_addto_recipe_sysroot () {
> >  }
> >  addtask addto_recipe_sysroot after do_populate_sysroot
> >  do_addto_recipe_sysroot[deptask] = "do_populate_sysroot"
> > +
> > +PATH:prepend = "${COREBASE}/scripts/cross-intercept:"
> > diff --git a/scripts/cross-intercept/ar b/scripts/cross-intercept/ar
> > new file mode 120000
> > index 00000000000..bc68ffd7a21
> > --- /dev/null
> > +++ b/scripts/cross-intercept/ar
> > @@ -0,0 +1 @@
> > +../native-intercept/ar
> > \ No newline at end of file
> > diff --git a/scripts/native-intercept/ar b/scripts/native-intercept/ar
> > new file mode 100755
> > index 00000000000..642b6eae864
> > --- /dev/null
> > +++ b/scripts/native-intercept/ar
> > @@ -0,0 +1,29 @@
> > +#!/usr/bin/env python3
> > +#
> > +# Wrapper around 'ar' that defaults to deterministic archives
> > +
> > +import os
> > +import shutil
> > +import sys
> > +
> > +# calculate path to the real 'ar'
> > +path = os.environ['PATH']
> > +path = path.replace(os.path.dirname(sys.argv[0]), '')
> > +real_ar = shutil.which('ar', path=path)
> > +
> > +if len(sys.argv) == 1:
> > +    os.execl(real_ar, 'ar')
> > +
> > +# modify args to mimic 'ar' configured with --default-deterministic-archives
> > +argv = sys.argv
> > +if argv[1].startswith('--'):
> > +    # No modifier given
> > +    None
> > +else:
> > +    # remove the optional '-'
> > +    if argv[1][0] == '-':
> > +        argv[1] = argv[1][1:]
> > +    argv[1] = argv[1].replace('u', '')
> > +    argv[1] = 'D' + argv[1]
> > +
> > +os.execv(real_ar, argv)
> > 
> 
> What should we do if the user is explicitly passing U, "Do not operate
> in deterministic mode." ? Don't replace any 'u' and don't pass D ?

That would seem to make sense to me...

Cheers,

Richard
Jacob Kroon Nov. 23, 2021, 11:13 p.m. UTC | #3
On 11/24/21 00:06, Richard Purdie wrote:
> On Tue, 2021-11-23 at 21:22 +0100, Jacob Kroon wrote:
>>
>> On 11/23/21 15:12, Richard Purdie wrote:
>>> From: Jacob Kroon <jacob.kroon@gmail.com>
>>>
>>> Add a wrapper around ar calls for native/cross recipes. This wrapper adds
>>> the -D option so that deterministic archives are built for native/cross
>>> output. This improves the changes of hash equivalence matches and hence
>>> build artefact reuse.
>>>
>>> We don't need this in the target case since we compile binutils-cross
>>> with an option making this the default. We need a wrapper since we need
>>> to remove the "u" option and replace it with "D" but also allow things like
>>> "--version" to continue to work too.
>>>
>>> Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
>>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>>> ---
>>>  meta/classes/cross.bbclass  |  2 ++
>>>  scripts/cross-intercept/ar  |  1 +
>>>  scripts/native-intercept/ar | 29 +++++++++++++++++++++++++++++
>>>  3 files changed, 32 insertions(+)
>>>  create mode 120000 scripts/cross-intercept/ar
>>>  create mode 100755 scripts/native-intercept/ar
>>>
>>> diff --git a/meta/classes/cross.bbclass b/meta/classes/cross.bbclass
>>> index 3e6a2f60b9e..9d951076a78 100644
>>> --- a/meta/classes/cross.bbclass
>>> +++ b/meta/classes/cross.bbclass
>>> @@ -93,3 +93,5 @@ python do_addto_recipe_sysroot () {
>>>  }
>>>  addtask addto_recipe_sysroot after do_populate_sysroot
>>>  do_addto_recipe_sysroot[deptask] = "do_populate_sysroot"
>>> +
>>> +PATH:prepend = "${COREBASE}/scripts/cross-intercept:"
>>> diff --git a/scripts/cross-intercept/ar b/scripts/cross-intercept/ar
>>> new file mode 120000
>>> index 00000000000..bc68ffd7a21
>>> --- /dev/null
>>> +++ b/scripts/cross-intercept/ar
>>> @@ -0,0 +1 @@
>>> +../native-intercept/ar
>>> \ No newline at end of file
>>> diff --git a/scripts/native-intercept/ar b/scripts/native-intercept/ar
>>> new file mode 100755
>>> index 00000000000..642b6eae864
>>> --- /dev/null
>>> +++ b/scripts/native-intercept/ar
>>> @@ -0,0 +1,29 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# Wrapper around 'ar' that defaults to deterministic archives
>>> +
>>> +import os
>>> +import shutil
>>> +import sys
>>> +
>>> +# calculate path to the real 'ar'
>>> +path = os.environ['PATH']
>>> +path = path.replace(os.path.dirname(sys.argv[0]), '')
>>> +real_ar = shutil.which('ar', path=path)
>>> +
>>> +if len(sys.argv) == 1:
>>> +    os.execl(real_ar, 'ar')
>>> +
>>> +# modify args to mimic 'ar' configured with --default-deterministic-archives
>>> +argv = sys.argv
>>> +if argv[1].startswith('--'):
>>> +    # No modifier given
>>> +    None
>>> +else:
>>> +    # remove the optional '-'
>>> +    if argv[1][0] == '-':
>>> +        argv[1] = argv[1][1:]
>>> +    argv[1] = argv[1].replace('u', '')
>>> +    argv[1] = 'D' + argv[1]
>>> +
>>> +os.execv(real_ar, argv)
>>>
>>
>> What should we do if the user is explicitly passing U, "Do not operate
>> in deterministic mode." ? Don't replace any 'u' and don't pass D ?
> 
> That would seem to make sense to me...
> 

Could we log a warning somewhere when this is done ? I can't write to
stdout here, but stderr perhaps ?

Jacob

Patch

diff --git a/meta/classes/cross.bbclass b/meta/classes/cross.bbclass
index 3e6a2f60b9e..9d951076a78 100644
--- a/meta/classes/cross.bbclass
+++ b/meta/classes/cross.bbclass
@@ -93,3 +93,5 @@  python do_addto_recipe_sysroot () {
 }
 addtask addto_recipe_sysroot after do_populate_sysroot
 do_addto_recipe_sysroot[deptask] = "do_populate_sysroot"
+
+PATH:prepend = "${COREBASE}/scripts/cross-intercept:"
diff --git a/scripts/cross-intercept/ar b/scripts/cross-intercept/ar
new file mode 120000
index 00000000000..bc68ffd7a21
--- /dev/null
+++ b/scripts/cross-intercept/ar
@@ -0,0 +1 @@ 
+../native-intercept/ar
\ No newline at end of file
diff --git a/scripts/native-intercept/ar b/scripts/native-intercept/ar
new file mode 100755
index 00000000000..642b6eae864
--- /dev/null
+++ b/scripts/native-intercept/ar
@@ -0,0 +1,29 @@ 
+#!/usr/bin/env python3
+#
+# Wrapper around 'ar' that defaults to deterministic archives
+
+import os
+import shutil
+import sys
+
+# calculate path to the real 'ar'
+path = os.environ['PATH']
+path = path.replace(os.path.dirname(sys.argv[0]), '')
+real_ar = shutil.which('ar', path=path)
+
+if len(sys.argv) == 1:
+    os.execl(real_ar, 'ar')
+
+# modify args to mimic 'ar' configured with --default-deterministic-archives
+argv = sys.argv
+if argv[1].startswith('--'):
+    # No modifier given
+    None
+else:
+    # remove the optional '-'
+    if argv[1][0] == '-':
+        argv[1] = argv[1][1:]
+    argv[1] = argv[1].replace('u', '')
+    argv[1] = 'D' + argv[1]
+
+os.execv(real_ar, argv)