fix(landoscript): ensure a newline exists between diff sections #1232
Conversation
|
Various tests of this have been done on https://treeherder.mozilla.org/jobs?repo=try&revision=b03bedac74ca99784b3ce0b855c5fbf7578fcef8 and https://treeherder.mozilla.org/jobs?repo=try&revision=8261e81eb89b378a5566d8b73a5c8977ea542642 ( |
Eijebong
left a comment
Eijebong
left a comment
There was a problem hiding this comment.
The patch makes sense, however while reviewing this, I found https://github.com/mozilla-releng/scriptworker-scripts/pull/1232/files#diff-11dcea3f6305f036dc56d6df14db892a02d6291a537bf52c6d5bff6143eb276bR247 which looks wrong, the second condition will always be true, probably should be: if f"-{before}" in diff and f"+{after}" in diff:?
I believe this broke with mozilla-releng#1203, where `diff_contents` (correctly) stopped always having a newline character at the end of a diff, but callers were not updated to handle it. This wasn't a problem in production because AFAICT, the only files we ever touch in landoscript that don't have an EOF newline is `CLOBBER`, which happens (luckily) to be the last file in the `merge_day` diff, and l10n-changesets, which are single file bumps. Unfortunately, this was masked by a bug in the tests where we split on `diff\n` instead of `\ndiff` - resulting in the diffed files not being split correctly.
Yes, indeed! |
bhearsum
force-pushed
the
push-rzyuvqszkpyx
branch
from
e1938df to
f9e08d3
Compare
2 participants
I believe this broke with #1203, where
diff_contents(correctly) stopped always having a newline character at the end of a diff, but callers were not updated to handle it.Unfortunately, this was masked by a bug in the tests where we split on
diff\ninstead of\ndiff- resulting in the diffed files not being split correctly.