Tails

#1 Updated by intrigeri 2018-03-16 17:54:29

• related to Bug #15400: Python files installed in /usr/local/lib break reproducibility added

#2 Updated by intrigeri 2018-03-16 17:54:44

• Status changed from Confirmed to In Progress
• % Done changed from 0 to 10

#3 Updated by intrigeri 2018-03-22 08:47:58

• Assignee changed from intrigeri to segfault
• QA Check changed from Ready for QA to Dev Needed

First, this branch was originally based on devel, but the one we merged for 3.6.1 had to be rebased on stable so the topic branch (still based on devel) now has duplicate commits. Please rebase it on top of stable and ensure there’s no duplicate commit.

Then, following-up on the beginning of the discussion that happened on Bug #15400:

segfault wrote:
> intrigeri wrote:
>> Thanks!
>>
>> * Code review:
>> wrt. commit:a867dfd898275d60c935c739a676e6ddb99e2847 I don’t really get it: why not drop the quotes instead? You’re not quoting the same variable a few lines above so it has to be safe for unquoted usage anyway.

> You are right. I thought the quotes in strip_nondeterminism_wrapper() in the following line were the cause:

strip-nondeterminism "${@}"

> But indeed in a quick test (I didn’t run the build yet) it seems to work if I just remove the quotes in 00-install-tailslib. I don’t understand why, but I’m used to not understanding why bash does weird things.

commit:b3429630495744cb9d88471c6389d340db399df2 LGTM.

>> I’m not super happy with the fact the implementation will break if the glob ever matches more than one file. Something based on ls or find and loops over the matches would be more robust.

> It should not break. I already tested this before, but I see that I should have noted it in the commit message: Both realpath and strip-nondeterminism can take multiple filenames, and then do its work on all of them. So if the glob matches multiple files, they will simply all be stripped.

OK, but -f only supports being passed exactly one path so it’ll break if the glob ever matches 0 or 2+ files. Try running this with /bin/sh:

package_glob_pattern="a*"
workdir=$(mktemp -d)
cd "$workdir"
touch a1 a2
[ ! -f ${package_glob_pattern} ]

… and you’ll see a shell runtime syntax error (“unexpected operator”).

#4 Updated by segfault 2018-03-25 11:39:56

• Assignee changed from segfault to intrigeri
• QA Check changed from Dev Needed to Ready for QA

> Please rebase it on top of stable and ensure there’s no duplicate commit.

Done.

> OK, but -f only supports being passed exactly one path so it’ll break if the glob ever matches 0 or 2+ files.

If the glob matches 0 files, it will not expand but remain as a pattern, so the -f test will work. But you’re right that it will break in the 2+ files case, I missed that. I pushed a commit which should fix that.

#5 Updated by intrigeri 2018-03-26 07:03:20

• Assignee changed from intrigeri to segfault
• % Done changed from 10 to 50
• QA Check changed from Ready for QA to Info Needed

OK, that’s better! But wait, what would be the drawback of the simpler:

    # Strip nondeterministic information from tailslib
    package_glob_pattern="/usr/local/lib/python3.*/dist-packages/Tailslib*.egg"
    strip_nondeterminism_wrapper --type zip ${package_glob_pattern} 2>/dev/null

?

#6 Updated by segfault 2018-03-26 16:20:31

• Assignee changed from segfault to intrigeri

intrigeri wrote:
> OK, that’s better! But wait, what would be the drawback of the simpler:
>
> […]
>
> ?

A less helpful error message in the case that the file is not found. Or do you only mean to replace this line?

strip_nondeterminism_wrapper --type zip "$(realpath ${package_glob_pattern})" 2>/dev/null

I guess that can be simplified, I don’t remember why I used realpath there. I’m also getting frustrated that we are still discussing these < 10 lines of shell script. Looking forward to when we ported the shell library to Python, so I can do things like this without making a bunch of errors / unnecessary complex things.

#8 Updated by intrigeri 2018-03-27 15:29:33

• Feature Branch changed from segfault:bug/15400-python-lib-breaks-reproducibility to bug/15400-python-lib-breaks-reproducibility

> intrigeri wrote:
>> OK, that’s better! But wait, what would be the drawback of the simpler:
>> […]
>> ?

> A less helpful error message in the case that the file is not found.

I see; that’s true but I don’t think it’s worth the hassle of getting the check right (given the time we’ve on it, it’s now very clear that it’s not a trivial task). I’ve pushed to our CI a few commits (based on your branch) that simplify this script a lot and if it works there I’ll reassign to anonym for merging.

> I’m also getting frustrated that we are still discussing these < 10 lines of shell script. Looking forward to when we ported the shell library to Python, so I can do things like this without making a bunch of errors / unnecessary complex things.

Fully agreed. As said elsewhere, I think it’s a waste of both your time and mine for me to mentor you through shell scripting in 2018. Now, of course the very script that installs our Python library cannot use it itself (and we would need strip_nondeterminism_wrapper in there to rewrite this script in Python). So you can now forget about this ticket!

#9 Updated by intrigeri 2018-03-27 17:58:20

• Assignee changed from intrigeri to anonym
• QA Check changed from Info Needed to Ready for QA

#10 Updated by bertagaz 2018-05-06 11:56:44

• Assignee changed from anonym to bertagaz

Taking this over, for it to get in 3.7 in time.

#11 Updated by bertagaz 2018-05-07 09:56:46

• Status changed from In Progress to Fix committed
• Assignee deleted (bertagaz)
• % Done changed from 50 to 100
• QA Check changed from Ready for QA to Pass

bertagaz wrote:
> Taking this over, for it to get in 3.7 in time.

And it looks good and passes the test suite so it’s merged.

#12 Updated by bertagaz 2018-05-10 10:59:51

• Status changed from Fix committed to Resolved