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

Cleaning-up Sidetrail's safety.patch to ensure that it applies cleanly #1810

Closed
wants to merge 1 commit into from

Conversation

aytey
Copy link

@aytey aytey commented Apr 24, 2020

Signed-off-by: Andrew V. Jones andrew.jones@vector.com

Please note that while we are transitioning from travis-ci to AWS CodeBuld, some tests are run on each platform. Non-AWS contributors will temporarily be unable to see CodeBuild results. We apologize for the inconvenience.

Issue

In s2n's master, the file safety.patch (from Sidetrail) does not cleanly apply:

$ git rev-parse --abbrev-ref HEAD
master
$ git rev-parse --short HEAD
75f7d991
$ git clean -f -d -x . && git checkout . && patch -p5 < tests/sidetrail/working/s2n-cbc/patches/safety.patch
Updated 0 paths from the index
patching file utils/s2n_safety.c
Hunk #1 succeeded at 57 (offset 2 lines).
patching file utils/s2n_safety.h
Hunk #1 succeeded at 23 (offset 2 lines).
Hunk #2 succeeded at 74 (offset 11 lines).
Hunk #3 FAILED at 81.
1 out of 3 hunks FAILED -- saving rejects to file utils/s2n_safety.h.rej

Description of changes

This PR regenerates (and fixes) safety.patch against master/75f7d991 such that the above now works:

$ git rev-parse --abbrev-ref HEAD
fix_safety_patch
$ git rev-parse --short HEAD
2b0ff08d
$ git clean -f -d -x . && git checkout . && patch -p5 < tests/sidetrail/working/s2n-cbc/patches/safety.patch
Updated 0 paths from the index
patching file utils/s2n_safety.c
patching file utils/s2n_safety.h

At least for me (GNU patch 2.7.6) the lines containing only whitespace (in the diff) are required, otherwise the patch only applies with "fuzz":

$ git clean -f -d -x . && git checkout . && sed -i "s#^\s*\$##g" tests/sidetrail/working/s2n-cbc/patches/safety.patch && patch -p5 < tests/sidetrail/working/s2n-cbc/patches/safety.patch
Removing utils/s2n_safety.c.orig
Updated 3 paths from the index
patching file utils/s2n_safety.c
Hunk #1 succeeded at 57 with fuzz 2.
patching file utils/s2n_safety.h

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
@aytey
Copy link
Author

aytey commented Apr 24, 2020

Ah, this PR is potentially useless ...

If you look at:

then this script does not refer to safety.patch, but to ../patches/safety1.patch and ../patches/safety2.patch (i.e., patches in the parent working directory and not in the s2n-cbc directory).

As far as I can tell, safety.patch is not used anywhere.

Shall we just delete this file then?

@aytey
Copy link
Author

aytey commented Apr 24, 2020

PR #1291 removed the use of safety.patch but did not remove the file itself: https://github.com/awslabs/s2n/pull/1291/files#diff-ca00e6f3b0997f5345181c4c2341ebf1L41

@camshaft
Copy link
Contributor

fixed in #1891

@camshaft camshaft closed this May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants