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.

Sign up for GitHub

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

Jump to bottom

Environment variable DEP_FOO_KEY is not generated for dev-dependencies #10927

Open
2ndTaleStudio opened this issue Aug 2, 2022 · 4 comments
Open

Environment variable DEP_FOO_KEY is not generated for dev-dependencies #10927

2ndTaleStudio opened this issue Aug 2, 2022 · 4 comments
Labels
A-build-scripts Area: build.rs scripts A-links Area: `links` native library links setting C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@2ndTaleStudio
Copy link

Problem

Creating a DEP_FOO_KEY environment variable in a build script is properly provided to all dependent crates if the crate providing this env-var is a usual dependency. However, as soon as this crate becomes a dev-dependency the DEP_FOO_KEY is no longer passed from the build script to the dependent crates.

This is unexpected behaviour.

Steps

  1. create crate FOO with a links section in it's cargo.toml and a build script that generates a DEP_FOO_KEY env variable
  2. create crate BAR that uses FOO as a dev-dependency.
  3. implement a build script for crate BAR and verify the available env-vars. The expected DEP_FOO_KEY is not present.

If in this example the crate FOO is maintained as a normal dependency of BAR the build script of BAR will show the expected env-var.

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.64.0-nightly (8827baaa7 2022-07-14)
release: 1.64.0-nightly
commit-hash: 8827baaa781b37872134c1ba692a6f0aeb37890e
commit-date: 2022-07-14
host: x86_64-pc-windows-msvc
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:Schannel)
os: Windows 10.0.22000 (Windows 10 Pro) [64-bit]
@2ndTaleStudio 2ndTaleStudio added the C-bug Category: bug label Aug 2, 2022
@2ndTaleStudio 2ndTaleStudio changed the title Environment variables DEP_FOO_KEY is not generated for dev-dependencies Environment variable DEP_FOO_KEY is not generated for dev-dependencies Aug 2, 2022
@epage
Copy link
Contributor

epage commented Aug 2, 2022

I believe dev-dependencies only impact the compilation of test binaries and not regular builds,. so environment variables injected by a dev-dependency should not show up for a [lib] build.

@2ndTaleStudio
Copy link
Author

Well, I do understand this fact, however, even in a dev/test build - e.g. while unit or integration tests are executed the build scripts do run but the DEP_FOO_KEY env var seems not be passed on. Sorry if I was not clear enough on this in my initial description.

@ehuss
Copy link
Contributor

ehuss commented Aug 13, 2022

Build scripts don't know about the different kinds of builds (tests, check, etc.). I think it is unlikely that it will be changed to run again when doing different things like commands that trigger the use of dev-dependencies. There are other issues for tracking that (like #4001 and #9394).

I'm going to leave this open, but I suspect it is not something that will get addressed without some major overhaul of how build scripts work.

@ehuss ehuss added A-build-scripts Area: build.rs scripts A-links Area: `links` native library links setting labels Aug 13, 2022
@ltratt ltratt mentioned this issue May 11, 2023
Closed
ltratt added a commit to ltratt/yk that referenced this issue May 11, 2023
@ltratt
Build ykllvm in target/<debug|release>/ykllvm.
97baa38 
Previously we tried building ykllvm in `OUT_DIR` but, unfortunately,
that means when testing we can't access the Cargo variable we defined in
rust-lang/cargo#10927. Previously, things
accidentally worked because we also required PATH to be set to
`/path/to/ykllvm/bin`, but that means you could pick up unexpected
versions of clang/clang++, for example.

This commit rethinks things. It builds a copy of ykllvm in
`target/<debug|release>/ykllvm`. [Technically this breaks Cargo's rules,
but since we're having to worm around a Cargo restriction, I think we
can deal with this: if Cargo's rules change in the future, we can change
the location easily enough.] `yk-config` gains `--cc` and `--cxx` flags
which give the user the path to the C/C++ compiler.

I've tried to keep this commit as minimal as possible: it would
certainly be possible to do various bits of cleaning-up at the same
time, but I think that would obscure what we're most interested in.
ltratt added a commit to ltratt/yk that referenced this issue May 11, 2023
@ltratt
Build ykllvm in target/<debug|release>/ykllvm.
0627984 
Previously we tried building ykllvm in `OUT_DIR` but, unfortunately,
that means when testing we can't access the Cargo variable we defined in
rust-lang/cargo#10927. Previously, things
accidentally worked because we also required PATH to be set to
`/path/to/ykllvm/bin`, but that means you could pick up unexpected
versions of clang/clang++, for example.

This commit rethinks things. It builds a copy of ykllvm in
`target/<debug|release>/ykllvm`. [Technically this breaks Cargo's rules,
but since we're having to worm around a Cargo restriction, I think we
can deal with this: if Cargo's rules change in the future, we can change
the location easily enough.] `yk-config` gains `--cc` and `--cxx` flags
which give the user the path to the C/C++ compiler.

I've tried to keep this commit as minimal as possible: it would
certainly be possible to do various bits of cleaning-up at the same
time, but I think that would obscure what we're most interested in.
@ltratt ltratt mentioned this issue May 11, 2023
Merged
ltratt added a commit to ltratt/yk that referenced this issue May 12, 2023
@ltratt
Build ykllvm in target/<debug|release>/ykllvm.
a6d4da4 
Previously we tried building ykllvm in `OUT_DIR` but, unfortunately,
that means when testing we can't access the Cargo variable we defined in
rust-lang/cargo#10927. Previously, things
accidentally worked because we also required PATH to be set to
`/path/to/ykllvm/bin`, but that means you could pick up unexpected
versions of clang/clang++, for example.

This commit rethinks things. It builds a copy of ykllvm in
`target/<debug|release>/ykllvm`. [Technically this breaks Cargo's rules,
but since we're having to worm around a Cargo restriction, I think we
can deal with this: if Cargo's rules change in the future, we can change
the location easily enough.] `yk-config` gains `--cc` and `--cxx` flags
which give the user the path to the C/C++ compiler.

I've tried to keep this commit as minimal as possible: it would
certainly be possible to do various bits of cleaning-up at the same
time, but I think that would obscure what we're most interested in.
ltratt added a commit to ltratt/yk that referenced this issue May 15, 2023
@ltratt
Build ykllvm in target/<debug|release>/ykllvm.
a65629f 
Previously we tried building ykllvm in `OUT_DIR` but, unfortunately,
that means when testing we can't access the Cargo variable we defined in
rust-lang/cargo#10927. Previously, things
accidentally worked because we also required PATH to be set to
`/path/to/ykllvm/bin`, but that means you could pick up unexpected
versions of clang/clang++, for example.

This commit rethinks things. It builds a copy of ykllvm in
`target/<debug|release>/ykllvm`. [Technically this breaks Cargo's rules,
but since we're having to worm around a Cargo restriction, I think we
can deal with this: if Cargo's rules change in the future, we can change
the location easily enough.] `yk-config` gains `--cc` and `--cxx` flags
which give the user the path to the C/C++ compiler.

One minor annoyance is that I've had to delete yksmp's tests, as that,
as written, they don't fit into cargo's model. We should bring these
back, but fixing them here feels like one thing too many.

I've tried to keep this commit as minimal as possible: it would
certainly be possible to do various bits of cleaning-up at the same
time, but I think that would obscure what we're most interested in.
@epage
Copy link
Contributor

epage commented Sep 20, 2023

What isn't too clear to me is why dev dependencies should be made available.

Depending on the answer to that, we might want to close this in favor of #4511

@epage epage added the S-triage Status: This issue is waiting on initial triage. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-build-scripts Area: build.rs scripts A-links Area: `links` native library links setting C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ehuss @epage @2ndTaleStudio