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

Tweak Layout to allow for non json file targets with internal "." #6255

Merged
merged 1 commit into from
Nov 25, 2018

Conversation

evq
Copy link
Contributor

@evq evq commented Nov 3, 2018

Per @alexcrichton 's comment in rust-lang/rust#55041 (comment), currently cargo assumes that a target with an internal . is a custom json target spec, using the file stem for the build directory name.

This PR changes cargo to only use the file stem for files with a json extension.

@alexcrichton
Copy link
Member

Thanks! To confirm, if you test this against the new target does everything work as expected?

@evq
Copy link
Contributor Author

evq commented Nov 15, 2018

Sorry for the delay - I had previously tested these changes with the custom json target equivalent of my rustc changes, but I wanted to test end to end with a rustc with my target built in.

I have now tested with custom xargo, custom cargo and custom rustc and everything works as expected up until I get this error:

[user@personal:~/source/atsaml11xxx]% LD_LIBRARY_PATH=/home/user/source/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib XARGO_RUST_SRC=/home/user/source/rust/src xargo build --release --target thumbv8m.base-none-eabi --example blink
   Compiling semver-parser v0.7.0
   Compiling proc-macro2 v0.4.20
   Compiling unicode-xid v0.1.0
   Compiling rand_core v0.3.0
   Compiling vcell v0.1.0
   Compiling cortex-m-semihosting v0.3.1 (https://github.com/rust-embedded/cortex-m-semihosting?rev=471378bc848172e12e22b9a3b83190747e70bf5c#471378bc)
   Compiling cortex-m v0.5.8 (https://github.com/rust-embedded/cortex-m?rev=b70a2179f4e8ee7908b61a6a97970e0cfcf2f313#b70a2179)
   Compiling cortex-m-rt v0.6.5 (https://github.com/rust-embedded/cortex-m-rt?rev=9cd01176a6a9985c9bc0040801ed593687dd6ab1#9cd01176)
   Compiling aligned v0.2.0
   Compiling atsaml11xxx v0.1.0 (/home/user/source/atsaml11xxx)
   Compiling r0 v0.2.2
   Compiling volatile-register v0.2.0
   Compiling rand_core v0.2.2
   Compiling semver v0.9.0
   Compiling rand v0.5.5
   Compiling rustc_version v0.2.3
   Compiling bare-metal v0.2.4
   Compiling quote v0.6.9
   Compiling panic-semihosting v0.5.1 (https://github.com/evq/panic-semihosting?rev=3d7041af24969f32606e5eb804e23eaac7fb0bfa#3d7041af)
   Compiling syn v0.15.18
   Compiling cortex-m-rt-macros v0.1.3 (https://github.com/rust-embedded/cortex-m-rt?rev=9cd01176a6a9985c9bc0040801ed593687dd6ab1#9cd01176)
thread '<unnamed>' panicked at 'procedural macro API is used outside of a procedural macro', libproc_macro/lib.rs:1314:13
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: custom attribute panicked
  --> examples/blink.rs:14:1
   |
14 | #[entry]
   | ^^^^^^^^
   |
   = help: message: procedural macro API is used outside of a procedural macro

error: aborting due to previous error

error: Could not compile `atsaml11xxx`.

To learn more, run the command again with --verbose.
[user@personal:~/source/atsaml11xxx]% ls target
release  thumbv8m.base-none-eabi

This doesn't seem related to my changes, but maybe something stands out to you?

@evq
Copy link
Contributor Author

evq commented Nov 19, 2018

I tested various configurations to try to narrow down the source of this issue - it seems to be related to the use of my local rustc build and not my cargo build.

Given that my only change to rustc was to add the thumbv8 baseline target I'm wondering if I'm missing something in my invocation above :/

@alexcrichton
Copy link
Member

@evq ah ok, perhaps it's be easiest to test if we merge this, integrate it, and see how it works out? We can always revert!

@evq
Copy link
Contributor Author

evq commented Nov 22, 2018

@alexcrichton turns out the error I was seeing above was because I was using a stage 1 build of rustc instead of stage 2. I missed the note that stage 1 has issues with procedural macros.

Everything is working as expected with a stage 2 rustc so this is good for merge from my end

@alexcrichton
Copy link
Member

@bors: r+

Ok!

@bors
Copy link
Contributor

bors commented Nov 25, 2018

📌 Commit a1f241a has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Nov 25, 2018

⌛ Testing commit a1f241a with merge 99bea3b...

bors added a commit that referenced this pull request Nov 25, 2018
Tweak Layout to allow for non json file targets with internal "."

Per @alexcrichton 's comment in rust-lang/rust#55041 (comment), currently cargo assumes that a target with an internal `.` is a custom json target spec, using the file stem for the build directory name.

This PR changes cargo to only use the file stem for files with a `json` extension.
@bors
Copy link
Contributor

bors commented Nov 25, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 99bea3b to master...

@bors bors merged commit a1f241a into rust-lang:master Nov 25, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Nov 27, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
Centril added a commit to Centril/rust that referenced this pull request Dec 1, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
bors added a commit to rust-lang/rust that referenced this pull request Dec 4, 2018
Update cargo, rls

26 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..5e85ba14aaa20f8133863373404cb0af69eeef2c
2018-11-15 19:13:04 +0000 to 2018-12-02 14:37:25 +0000
- ConflictStoreTrie: Faster filtered search (rust-lang/cargo#6366)
- Remove `cmake` as a requirement (rust-lang/cargo#6368)
- progress: display "Downloading 1 crate" instead of "Downloading 1 crates" (rust-lang/cargo#6369)
- Use expect over unwrap, for panic-in-panic aborts (rust-lang/cargo#6364)
- Switch to pretty_env_logger, under --features pretty-env-logger (rust-lang/cargo#6362)
- use allow-dirty option in `cargo package` to skip vcs checks (rust-lang/cargo#6280)
- remove clones made redundant by Intern PackageId (rust-lang/cargo#6352)
- docs: correct profile usage of `cargo test --release` (rust-lang/cargo#6345)
- Improve doc for `cargo install` (rust-lang/cargo#6354)
- Intern PackageId (rust-lang/cargo#6332)
- Clean only release artifacts if --release option is set (rust-lang/cargo#6349)
- remove clones made redundant by Intern SourceId (rust-lang/cargo#6347)
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
kennytm added a commit to kennytm/rust that referenced this pull request Dec 7, 2018
Add Armv8-M Mainline targets

This commit enables the Armv8-M Mainline architecture profile.
It adds two targets:
  - `thumbv8m.main-none-eabi`
  - `thumbv8m.main-none-eabihf`

The second one uses the Floating Point Unit for floating point
operations. It mainly targets the Cortex-M33 processor, which
can have the optional Floating Point Unit extension.

It follows rust-lang#55041 which does it for Baseline. I will rebase this branch on top of it when it is merged to not create conflicts as we have some files in common. To make it work, it still relies on the Cargo change to be merged (accepting "." in target names, rust-lang/cargo#6255).

The goal would also be to add this target in the CI so that the `core` library is available for everybody. To do this, some changes will be needed to compile successfully the needed libraries:

* `cc-rs` needs to be updated to allow compiling C code for Armv8-M architectures profiles. It is only a few lines to add [here](https://github.com/alexcrichton/cc-rs/blob/a76611ad9836fa8c44fa8220a1d2a96dd3b7d4b6/src/lib.rs#L1299).
* Some assembly files in `builtins` in `compiler-rt` were not assembling for Armv8-M Mainline. I sent changes [upstream](https://reviews.llvm.org/D51854) to that project to fix that. The Rust version of `compiler-rt` will have to be updated to contain [that commit](llvm-mirror/compiler-rt@a34cdf8).

I tested it using the [Musca-A Test Chip board](https://developer.arm.com/products/system-design/development-boards/iot-test-chips-and-boards/musca-a-test-chip-board) but more intensively on the [Armv8-M FVP](https://developer.arm.com/products/system-design/fixed-virtual-platforms) (emulation platform). I am going to try to release my test code soon, once I tidy it up 👍
@ehuss ehuss added this to the 1.32.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.

4 participants