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

Build proc-macro-test-impl out-of-tree #12831

Merged
merged 5 commits into from
Jul 20, 2022

Conversation

fasterthanlime
Copy link
Contributor

Building it in-place fails in rust CI because the source directory is read-only. This changes proc-macro-test's build script to first
copy imp under OUT_DIR (which is read-write).

It also prints stdout/stderr for the nested cargo invocation, should it fail. (I've seen failures in rust CI that I couldn't explain, and
when they take 25 minutes to reproduce, you want to have that info)

This change is tracked in:

Maintainer impact: none.

Building it in-place fails in rust CI because the source directory
is read-only. This changes `proc-macro-test`'s build script to first
copy `imp` under `OUT_DIR` (which is read-write).

It also prints stdout/stderr for the nested cargo invocation, should
it fail. (I've seen failures in rust CI that I couldn't explain, and
when they take 25 minutes to reproduce, you want to have that info)
@fasterthanlime
Copy link
Contributor Author

Tested locally with:

$ cd crates/proc-macro-test
$ sudo chown root:root -R ./imp
$ (cd imp && cargo build)
error: Permission denied (os error 13) at path "/home/amos/bearcove/rust-analyzer/crates/proc-macro-test/imp/targetC0YSqJ"
$ cargo clean -q && cargo test -v
(tests pass)

@Veykril
Copy link
Member

Veykril commented Jul 20, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

📌 Commit 898898d has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

⌛ Testing commit 898898d with merge 72aa862...

bors added a commit that referenced this pull request Jul 20, 2022
…eykril

Build proc-macro-test-impl out-of-tree

Building it in-place fails in rust CI because the source directory is read-only. This changes `proc-macro-test`'s build script to first
copy `imp` under `OUT_DIR` (which is read-write).

It also prints stdout/stderr for the nested cargo invocation, should it fail. (I've seen failures in rust CI that I couldn't explain, and
when they take 25 minutes to reproduce, you want to have that info)

This change is tracked in:

  * #12818

Maintainer impact: none.
fasterthanlime and others added 2 commits July 20, 2022 16:56
Co-authored-by: Laurențiu Nicola <lnicola@users.noreply.github.com>
Co-authored-by: Laurențiu Nicola <lnicola@users.noreply.github.com>
@fasterthanlime
Copy link
Contributor Author

@Veykril this needs another r+, @lnicola caught some typos in comments and other commits snuck in

@lnicola
Copy link
Member

lnicola commented Jul 20, 2022

@bors delegate+

In case you remove the temporary directory at the end (you might also need to chdir to the previous directory if you do that).

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

✌️ @fasterthanlime can now approve this pull request

@fasterthanlime
Copy link
Contributor Author

In case you remove the temporary directory at the end (you might also need to chdir to the previous directory if you do that).

It's under OUT_DIR, so cargo clean will take care of it. We also overwrite it on every build where proc-macro-test-impl changes. It's also very small.

I'm gonna invoke "we might want to investigate that dir after a build" to not have to add additional cleanups 😄

(If it was under std::env::temp_dir() we definitely would want to clean it up, that would add up)

@fasterthanlime
Copy link
Contributor Author

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

📌 Commit 844aa8b has been approved by fasterthanlime

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

⌛ Testing commit 844aa8b with merge 7dc36ee...

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

☀️ Test successful - checks-actions
Approved by: fasterthanlime
Pushing 7dc36ee to master...

@bors bors merged commit 7dc36ee into rust-lang:master Jul 20, 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