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

feat(build-plan): output invocations after running build scripts #7123

Closed
wants to merge 1 commit into from
Closed

feat(build-plan): output invocations after running build scripts #7123

wants to merge 1 commit into from

Conversation

schomatis
Copy link
Contributor

Without any feedback on the original issue #7068 I'm going ahead and submitting this PR to flesh out the details with the actual code, but feel free to close it if this wasn't an acceptable change in the first place.

The main change in the current architecture (and whose rationale should be reviewed in more depth than the rest of the code) is the inclusion of the new to_be_compiled attribute directly in UnitInner (and not in one of its subcomponents like CompileMode or Profile).


feat(build-plan): output invocations after running build scripts

Extend the `--build-plan` option with named value `STAGE` that indicates at
which point in the build process to output the (remaining) invocations. The (up
to now) only behavior is labeled as the `init` stage (compile nothing, add all
invocations to the build plan) and a new stage is added, `post-build-scripts`,
to compile and run custom build scripts (along with their dependencies) and
generate the build plan with the invocations of the remaining units (that were
actually going to be compiled and linked in the final binary/library).

Add a new attribute to `Unit` (actually `UnitInner`), `to_be_compiled`, to
signal (depending on the build plan selected, if any) which unit will be
compiled and which added to the build plan (see its documentation for the full
rationale of its inclusion in `Unit`). Replace the binary logic of local
`build_plan: bool` variables, signaling if the `--build-plan` option is used,
with the `to_be_compiled` attribute that contains a more fine-grained logic to
decide which unit will be compiled (based on the build plan selected). This
change is seen in the diff as (roughly) the replacement of `build_plan` (now
deprecated) with the negated value (but same meaning) in `!to_be_compiled`.

Add two tests for the new stage, and modify the basic test
(`cargo_build_plan_simple`) to explicitly include the stage name `init` to test
the new label (the rest of the build plan tests remain the same, to check
backwards compatibility with the previous `--build-plan`-only use).

Closes #7068.

Extend the `--build-plan` option with named value `STAGE` that indicates at
which point in the build process to output the (remaining) invocations. The (up
to now) only behavior is labeled as the `init` stage (compile nothing, add all
invocations to the build plan) and a new stage is added, `post-build-scripts`,
to compile and run custom build scripts (along with their dependencies) and
generate the build plan with the invocations of the remaining units (that were
actually going to be compiled and linked in the final binary/library).

Add a new attribute to `Unit` (actually `UnitInner`), `to_be_compiled`, to
signal (depending on the build plan selected, if any) which unit will be
compiled and which added to the build plan (see its documentation for the full
rationale of its inclusion in `Unit`). Replace the binary logic of local
`build_plan: bool` variables, signaling if the `--build-plan` option is used,
with the `to_be_compiled` attribute that contains a more fine-grained logic to
decide which unit will be compiled (based on the build plan selected). This
change is seen in the diff as (roughly) the replacement of `build_plan` (now
deprecated) with the negated value (but same meaning) in `!to_be_compiled`.

Add two tests for the new stage, and modify the basic test
(`cargo_build_plan_simple`) to explicitly include the stage name `init` to test
the new label (the rest of the build plan tests remain the same, to check
backwards compatibility with the previous `--build-plan`-only use).
@rust-highfive
Copy link

r? @alexcrichton

(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 Jul 12, 2019
// FIXME: The JSON matching pattern in this test is very brittle, we just care
// about the above *extra* items in the `"args"` and `"env"` arrays, but we need
// to add *all* their elements for the pattern to match, there doesn't seem to
// be a more flexible wildcard of the sort `{... "extra-item"}`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing for non-Linux targets, I'll try to implement a wildcard of the sort.

@bors
Copy link
Collaborator

bors commented Jul 19, 2019

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

@alexcrichton
Copy link
Member

Sorry for the delay and lack of messaging here @schomatis. Unfortunately though the build-plan mechanism is pretty lackluster in Cargo and is full of a lot of bugs and doesn't have a great plan for moving forward.

I'm going to close this because I don't think we're going to take major changes to the build-plan infrastructure because it needs to sort of start from scratch at this point. We're hopeful that we can find some time to set the direction for build plan in the future, but unfortunately it will require a relatively large investment on the team's part to lay out the current pros/cons and how we imagine them getting fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output build plan after running custom build scripts
4 participants