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

Fix the unit dependency graph with dev-dependency links #8969

Merged
merged 1 commit into from
Dec 12, 2020

Conversation

alexcrichton
Copy link
Member

This commit fixes #8966 by updating the unit generation logic to avoid
generating an erroneous circular dependency between the execution of two
build scripts. This has been present for Cargo in a long time since #5651
(an accidental regression), but the situation appears rare enough that
we didn't get to it until now!

Closes #8966

This commit fixes rust-lang#8966 by updating the unit generation logic to avoid
generating an erroneous circular dependency between the execution of two
build scripts. This has been present for Cargo in a long time since rust-lang#5651
(an accidental regression), but the situation appears rare enough that
we didn't get to it until now!

Closes rust-lang#8966
@rust-highfive
Copy link

r? @ehuss

(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 Dec 11, 2020
@ehuss
Copy link
Contributor

ehuss commented Dec 12, 2020

Thanks for looking into this! Sorry that #5651 seems to have caused so many problems.

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 12, 2020

📌 Commit 4761ada has been approved by ehuss

@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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@bors
Copy link
Collaborator

bors commented Dec 12, 2020

⌛ Testing commit 4761ada with merge dd41106...

@bors
Copy link
Collaborator

bors commented Dec 12, 2020

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing dd41106 to master...

@bors bors merged commit dd41106 into rust-lang:master Dec 12, 2020
@alexcrichton alexcrichton deleted the fix-cycle branch December 12, 2020 19:01
@alexcrichton
Copy link
Member Author

Heh no worries! Not like it's your fault!

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2020
Update cargo

4 commits in d274fcf862b89264fa2c6b917b15230705257317..a3c2627fbc2f5391c65ba45ab53b81bf71fa323c
2020-12-07 23:08:44 +0000 to 2020-12-14 17:21:26 +0000
- Check if rerun-if-changed points to a directory. (rust-lang/cargo#8973)
- Implement external credential process. (RFC 2730) (rust-lang/cargo#8934)
- Revert recent build-std vendoring changes (rust-lang/cargo#8968)
- Fix the unit dependency graph with dev-dependency `links` (rust-lang/cargo#8969)
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Jan 12, 2021
If a package is tested and the library for the package wasn't built
(e.g. only tested or it wasn't present) then the `links` env vars from
dependencies weren't showing up to the build script by accident. This
was an accidental regression from rust-lang#8969.

The intention of rust-lang#8969 was to exclude connections to build scripts
connected via dev-dependencies, but it only applied a heuristic because
the unit graph doesn't retain information about dev-dependencies. The
fix here is to instead actually retain information about
dev-dependencies which is only used for constructing the unit graph and
connecting build script executions to one another.

Closes rust-lang#9063
bors added a commit that referenced this pull request Jan 12, 2021
Fix `links` vars showing up for testing packages

If a package is tested and the library for the package wasn't built
(e.g. only tested or it wasn't present) then the `links` env vars from
dependencies weren't showing up to the build script by accident. This
was an accidental regression from #8969.

The intention of #8969 was to exclude connections to build scripts
connected via dev-dependencies, but it only applied a heuristic because
the unit graph doesn't retain information about dev-dependencies. The
fix here is to instead actually retain information about
dev-dependencies which is only used for constructing the unit graph and
connecting build script executions to one another.

Closes #9063
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Jan 12, 2021
If a package is tested and the library for the package wasn't built
(e.g. only tested or it wasn't present) then the `links` env vars from
dependencies weren't showing up to the build script by accident. This
was an accidental regression from rust-lang#8969.

The intention of rust-lang#8969 was to exclude connections to build scripts
connected via dev-dependencies, but it only applied a heuristic because
the unit graph doesn't retain information about dev-dependencies. The
fix here is to instead actually retain information about
dev-dependencies which is only used for constructing the unit graph and
connecting build script executions to one another.

Closes rust-lang#9063
@ehuss ehuss added this to the 1.50.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Stack overflow if dev dependency has links
4 participants