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

Add an unstable option to build proc macros for both the host and the target #6547

Merged
merged 2 commits into from
Feb 19, 2019

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Jan 12, 2019

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 13, 2019

Sorry, can you help me understand when this would be needed?

@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 13, 2019

This is needed when you want to cross compile a library for a target A which uses a proc macro crate. Currently we'll only compile the proc macro crate for the host, which means the proc macro crate won't be usable if we actually run the compiler on target A and we try to use our cross compiled library. Note that our library can reexport the macros from the proc macro crate, so it needs to be available to run on the target.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 14, 2019

So I think I am still not understanding. You are cross compiling a rust library (not a final artifact) so that the target can use that file as part of a rust build? There is no rust abi. I am surprised that it works at all.

The workflow I would have expected are to:

  • Do all development on the host. Cross compile the final artifacts (executables or c compatible libraries) to the target. Copy and run. No need for proc macros to run on the target.
  • Do all compilation on the target. Copy source to the target. Compile locally and run. No need for cross compile.

But I do not know much about the uses of cross compiles, can you explain why you are doing this workflow?

@alexcrichton
Copy link
Member

@Eh2406 I think a missing piece of information is that this is for the compiler itself. The compiler gets a free pass with unstable features because it deals with its own breakage, and I suspect that this feature would be unlikely to ever graduate from unstable status.

@Zoxc I think before merging this it'd be good to get a working build first. This may not be all that's needed on Cargo's end, so could this be used with a local build of Cargo to test out support in rustc?

@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 14, 2019

@alexcrichton I should be able to test these change locally, but I was hoping to sneak this into the upcoming release since I can't actually land changes in rustc until this is in beta.

@alexcrichton
Copy link
Member

We can backport this for rustc to unblock work there, so I'm not too worried about the scheduling here

@bors
Copy link
Collaborator

bors commented Jan 20, 2019

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

@alexcrichton
Copy link
Member

@Zoxc is this ready to go for rust-lang/rust#58013? If so I think this is fine to land on this branch and beta to get it in for the compiler

@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 16, 2019

This should be good to go

@mati865
Copy link
Contributor

mati865 commented Feb 18, 2019

Friendly ping, branching is going to happen soon.

@vext01
Copy link
Contributor

vext01 commented Feb 19, 2019

I'm working on a research project where I need to use serde in the compiler.

It'd be neat if this could be merged soon, so that #58013 can be unblocked.

Thanks :)

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Feb 19, 2019

📌 Commit 2a95bd0 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Feb 19, 2019

⌛ Testing commit 2a95bd0 with merge 7fd9c73...

bors added a commit that referenced this pull request Feb 19, 2019
Add an unstable option to build proc macros for both the host and the target

r? @alexcrichton
@bors
Copy link
Collaborator

bors commented Feb 19, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 7fd9c73 to master...

@bors bors merged commit 2a95bd0 into rust-lang:master Feb 19, 2019
@Zoxc Zoxc deleted the dual-proc-macros branch February 21, 2019 16:03
bors added a commit to rust-lang/rust that referenced this pull request Feb 21, 2019
Update cargo

9 commits in 865cb70106a6b1171a500ff68f93ab52eea56e72..b33ce7fc9092962b0657b4c25354984b5e5c47e4
2019-02-10 15:49:37 +0000 to 2019-02-19 18:42:50 +0000
- Don't retry invalid credentials from git credential helpers (rust-lang/cargo#6681)
- Fix some typos in resolver tests (rust-lang/cargo#6682)
- Add an unstable option to build proc macros for both the host and the target (rust-lang/cargo#6547)
- Test cases proving RUSTC_WRAPPER can be a relative path (rust-lang/cargo#6638)
- Add support for Azure DevOps (rust-lang/cargo#6264)
- Update docs for removed `patch` restriction. (rust-lang/cargo#6663)
- Fix incorrect help message (rust-lang/cargo#6555)
- Stabilize Alternative Registries (rust-lang/cargo#6654)
- Having a [patch] section when publishing is not an error (rust-lang/cargo#6535)
vext01 added a commit to vext01/ykrustc_pv that referenced this pull request Feb 22, 2019
@ehuss ehuss added this to the 1.34.0 milestone Feb 6, 2022
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

8 participants