Skip to content

[apidiff] Fix a few issues with the apidiff when the mono hash or api diff url changes. #6084

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.

Sign up for GitHub

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

Jump to bottom
Merged
rolfbjarne merged 5 commits into from
May 22, 2019
Merged

[apidiff] Fix a few issues with the apidiff when the mono hash or api diff url changes. #6084

rolfbjarne merged 5 commits into from
May 22, 2019

Conversation

@rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented May 20, 2019 •
edited
Loading

  • Change the name of the unzip stamp and download dir to contain the hash.
  • Add rule to get mono-api-info.exe and mono-api-html.exe.
  • Make make not delete temporary files.

rolfbjarne added 3 commits May 20, 2019 17:55
@rolfbjarne
[apidiff] Add rule to get mono-api-info.exe and mono-api-html.exe.
d5ffc34 
This fixes an issue when bumping mono: when bumping mono, the already
downloaded mono archive isn't applicable (because we've changed to the
previous commit, which has the previous mono hash, whose archive hasn't been
downloaded).

So add a rule to get mono-api-info.exe and mono-api-html.exe, by downloading
the current mono archive.
@rolfbjarne
[apidiff] Change the name of the unzip stamp and download dir to cont…
abb0b95 
…ain the hash.

This way the logic doesn't get confused when the hash changes (or there's an
old unzip stamp in the directory), and things are downloaded again as
expected.
@rolfbjarne
[apidiff] Make make not delete temporary files.
8623663 
Things end up confused if temporary files have been removed by make, but our
stamp file that the temporary files are still there is present.
@rolfbjarne rolfbjarne added the skip-all-tests Skip all the tests label May 20, 2019
@monojenkins
Copy link
Collaborator

monojenkins commented May 20, 2019

Build success
✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
✅ Generator Diff (no change)
✅ Test run succeeded

@rolfbjarne
Copy link
Member Author

rolfbjarne commented May 20, 2019

This isn't quite right, the api diff should be empty, not report breaking changes 😒

rolfbjarne added 2 commits May 21, 2019 18:24
@rolfbjarne
[apidiff] No need to make everything depend on the bundled zip.
0fcec15 
If everything that needs the bundled zip already depends on it.
@rolfbjarne
[apidiff] Restore original hash before calculating api diff.
55b4f87 
This makes it less annoying when the api diff calculation changes, because
with the previous behavior they were impossible to test in a PR, since any
changes wouldn't take effect until after the PR was merged.
@rolfbjarne rolfbjarne mentioned this pull request May 21, 2019
Merged
@monojenkins
Copy link
Collaborator

monojenkins commented May 21, 2019

Build success
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
✅ Test run succeeded

@rolfbjarne rolfbjarne merged commit 4d4215e into dotnet:master May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@spouliot spouliot spouliot approved these changes

@mandel-macaque mandel-macaque mandel-macaque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip-all-tests Skip all the tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rolfbjarne @monojenkins @spouliot @mandel-macaque