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

diff binaries as part of CI #19805

Merged
merged 3 commits into from Jan 18, 2022
Merged

Conversation

peterbarker
Copy link
Contributor

This will allow us to be assured that there's no changes from a PR, at least for things we test in CI; if the binaries are identical the functionality is identical.

I've tested the changes to the waf build by creating another commit on top of this PR and ensured that the output files have the same checksum before/after (they don't on master).

The binaries on this PR won't match master (i.e. the diff will show differences) as master doesn't have the patches to use a known git hash.

building AP_Periph requires these to be able to be converted into a
number
This allows for reproducible builds to be produced
If a change is not supposed to change build outcomes this will help show
it
Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow is only to validate build size, so instead of build twice, let's switch to only no_git version

@peterbarker
Copy link
Contributor Author

@khancyr that would save about 34 seconds per item in the matrix.

I put the timings into this PR so we could debate just this point :-)

I didn't want to modify the original test (build a board just as a normal person would), as it's a really base-line test which we don't want to break. It's possible but not likely that we could break the builds if you didn't specify these git hashes.

The other reason I left the includes-real-git-hashes build in is that you can download the binaries as artifacts, and I think it would be important at that point to have the git hash in them so you can recognise them and where they came from later.

@tridge tridge merged commit e0d1fd1 into ArduPilot:master Jan 18, 2022
@peterbarker peterbarker deleted the pr/diff-binaries branch January 18, 2022 01:25
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.

None yet

4 participants