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

Dual proc macro hash #65698

Merged
merged 2 commits into from
Nov 1, 2019
Merged

Dual proc macro hash #65698

merged 2 commits into from
Nov 1, 2019

Conversation

msizanoen1
Copy link
Contributor

This PR changes current -Z dual-proc-macro mechanism from resolving only by name to including the hash of the host crate inside the transistive dependency information to prevent name conflicts.
Fix partially #62558

@rust-highfive
Copy link
Collaborator

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 22, 2019
@msizanoen1
Copy link
Contributor Author

r? @Xanewok

@rust-highfive rust-highfive assigned Xanewok and unassigned zackmdavis Oct 22, 2019
@petrochenkov petrochenkov self-assigned this Oct 22, 2019
@msizanoen1 msizanoen1 force-pushed the dual-proc-macro-hash branch from 5223720 to f25624d Compare October 23, 2019 04:36
@mati865
Copy link
Contributor

mati865 commented Oct 23, 2019

Cc @Zoxc

@msizanoen1
Copy link
Contributor Author

@alexcrichton Can you run a try build?

@mati865

This comment has been minimized.

@msizanoen1
Copy link
Contributor Author

msizanoen1 commented Oct 24, 2019

I have already tested this locally. RLS and Rustfmt builds and Miri is failing because of unrelated issues (#65684). Clippy for cross platforms is still waiting on rust-lang/rust-clippy#4714

@bors
Copy link
Contributor

bors commented Oct 25, 2019

☔ The latest upstream changes (presumably #65771) made this pull request unmergeable. Please resolve the merge conflicts.

@msizanoen1 msizanoen1 force-pushed the dual-proc-macro-hash branch from f25624d to 2f46e0b Compare October 25, 2019 06:21
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-25T06:21:40.4132676Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-25T06:21:40.4353147Z ##[command]git config gc.auto 0
2019-10-25T06:21:40.4438961Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-25T06:21:40.4500613Z ##[command]git config --get-all http.proxy
2019-10-25T06:21:40.4630692Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65698/merge:refs/remotes/pull/65698/merge
---
2019-10-25T06:39:01.8104364Z   local time: Fri Oct 25 06:39:01 UTC 2019
2019-10-25T06:39:02.0900837Z   network time: Fri, 25 Oct 2019 06:39:02 GMT
2019-10-25T06:39:02.0900958Z == end clock drift check ==
2019-10-25T06:39:05.2421460Z 
2019-10-25T06:39:05.2522640Z ##[error]Bash exited with code '1'.
2019-10-25T06:39:05.2616878Z ##[section]Starting: Checkout
2019-10-25T06:39:05.2618735Z ==============================================================================
2019-10-25T06:39:05.2618781Z Task         : Get sources
2019-10-25T06:39:05.2618822Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@msizanoen1
Copy link
Contributor Author

r? @Zoxc

@rust-highfive rust-highfive assigned Zoxc and unassigned Xanewok and petrochenkov Oct 26, 2019
@petrochenkov
Copy link
Contributor

Wait, I've just got to reviewing this.

@petrochenkov petrochenkov self-assigned this Oct 26, 2019
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2019
@petrochenkov
Copy link
Contributor

r? @petrochenkov

@petrochenkov
Copy link
Contributor

r=me with commits squashed

@msizanoen1
Copy link
Contributor Author

How do I squash commits?

@tesuji
Copy link
Contributor

tesuji commented Oct 26, 2019

Use git rebase -i master and follow instructions.
See https://github.com/wprig/wprig/wiki/How-to-squash-commits for more details.

@petrochenkov
Copy link
Contributor

(I usually just soft-reset to the first commit and then amend everything else into it, it's pretty convenient to do from git gui.)

@Zoxc
Copy link
Contributor

Zoxc commented Oct 29, 2019

You could try cherry picking this PR just as a sanity check that things work: #56447

You can also do the CI cross compilation docker builds locally to ensure stuff works before trying to land this, if you haven't done so already.

@petrochenkov
Copy link
Contributor

@msizanoen1
Could you notify me when you finish testing and making changes?

@msizanoen1
Copy link
Contributor Author

msizanoen1 commented Oct 30, 2019

@petrochenkov Testing done - all green tools on toolstate builds (except clippy - rust-lang/rust-clippy#4714) after introducing serde-derive dependency on rustc.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 30, 2019

📌 Commit 03e7964cd9a2d6ff89ecb0fb9b3ea5b066b5ff39 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2019
@bors
Copy link
Contributor

bors commented Oct 31, 2019

☔ The latest upstream changes (presumably #65990) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 31, 2019
This changes the mechanism of `-Z dual-proc-macro` to record the host
proc macro hash in the transistive dependency information and use it
during dependency resolution instead of resolving only by name.
@msizanoen1 msizanoen1 force-pushed the dual-proc-macro-hash branch from 03e7964 to 8a0d233 Compare October 31, 2019 06:12
@msizanoen1
Copy link
Contributor Author

@petrochenkov Rebased for conflict resolution.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 1, 2019

📌 Commit 8a0d233 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 1, 2019
@bors
Copy link
Contributor

bors commented Nov 1, 2019

⌛ Testing commit 8a0d233 with merge 253fc0e...

bors added a commit that referenced this pull request Nov 1, 2019
Dual proc macro hash

This PR changes current `-Z dual-proc-macro` mechanism from resolving only by name to including the hash of the host crate inside the transistive dependency information to prevent name conflicts.
Fix partially #62558
@bors
Copy link
Contributor

bors commented Nov 1, 2019

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 253fc0e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 1, 2019
@bors bors merged commit 8a0d233 into rust-lang:master Nov 1, 2019
@msizanoen1 msizanoen1 deleted the dual-proc-macro-hash branch November 1, 2019 10:32
@msizanoen1 msizanoen1 restored the dual-proc-macro-hash branch July 7, 2022 15:39
@msizanoen1 msizanoen1 deleted the dual-proc-macro-hash branch July 7, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants