Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable reproducible workflow #181

Merged
merged 2 commits into from
Oct 13, 2020
Merged

Conversation

ia0
Copy link
Member

@ia0 ia0 commented Oct 9, 2020

As illustrated by #180, our build is not hermetic. As a consequence this workflow fails for unrelated reasons. Disabling until it brings any value.

@google-cla google-cla bot added the cla: yes label Oct 9, 2020
@ia0 ia0 requested a review from gendx October 9, 2020 10:17
@ia0 ia0 requested a review from jmichelp October 12, 2020 08:53
@gendx
Copy link
Collaborator

gendx commented Oct 12, 2020

Quoting the rationale in #180 (comment):

The context is that the workflow for reproducibility is thus completely useless, because it tracks noise instead of signal. As such it should be disabled until this is fixed.

I'd argue that the current workflow tracks signal+noise. Therefore "completely useless" is too strong a statement IMO - as the actual signal affects it.

One can argue that the core reproducibility is actually quite stable, i.e. it's really rare that a pull request makes the build totally unreproducible. By that I mean removing some flag like --remap-path-prefix:

"--remap-path-prefix={}=".format(os.getcwd()),

So we mostly observe noise, which is indeed a problem.

But I think we should improve the process to reduce the noise (e.g. #151) rather than outright removing any reproducibility goal.

@ia0
Copy link
Member Author

ia0 commented Oct 12, 2020

So we mostly observe noise, which is indeed a problem.

Yes, that's what I mean by completely useless. A workflow should have a low rate of false positives otherwise it hinders engineering velocity much more than it has benefits. Here, there are no true positives and only false positives on almost every commit including commits that do not affect the build.

But I think we should improve the process to reduce the noise

Yes obviously. This PR is a temporary measure until #182 is fixed. Then #151 is a possible improvement on the workflow system.

@jmichelp
Copy link
Collaborator

I would suggest that either we decide that compiling/generating the build artefacts with this workflow are useful and then we continue running all the commands but we ensure this can't fail (e.g. git --diff ... || true) or we disable completely the workflow.

@ia0
Copy link
Member Author

ia0 commented Oct 13, 2020

The git diff || true seems strictly better because we get more information which could be used for debugging. Although this information can be recovered from the artifacts which are still produced, I still did the change.

Once debugging is not needed because #182 is fixed, we can either re-enable failing on diffs, or continue with the improvements suggested in #151 instead.

@ia0 ia0 merged commit 0aece24 into google:master Oct 13, 2020
@ia0 ia0 deleted the disable_reproducible_workflow branch October 13, 2020 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants