Add a way to not build tier one targets by default #1145
Conversation
jyn514
added
S-waiting-on-review
Do you need this flexibility? We certainly don't in docs.rs and it would make the implementation more complicated. |
|
Well, that sounds like a fairly natural need. I'm not sure that I personally need this but I think it can be helpful. |
|
I think I prefer to hold off on implementing that, then. Ideally we wouldn't have this variable at all and you'd always have to specify your targets, but that would be a breaking change: https://internals.rust-lang.org/t/pre-rfc-build-only-one-target-by-default-on-docs-rs/12804. So I definitely don't want to expand this feature if I don't have to. |
|
I would invert the variable, something like |
|
Ok, I updated it in the metadata library but not in docs.rs, since I'd like 'no variable set' to match |
src/config.rs
Outdated
| @@ -87,6 +87,7 @@ impl Config { | |||
| local_docker_image: maybe_env("DOCS_RS_LOCAL_DOCKER_IMAGE")?, | |||
| toolchain: env("CRATESFYI_TOOLCHAIN", "nightly".to_string())?, | |||
| build_cpu_limit: maybe_env("DOCS_RS_BUILD_CPU_LIMIT")?, | |||
| only_build_default_target: env("DOCSRS_ONLY_BUILD_DEFAULT_TARGET", false)?, | |||
There was a problem hiding this comment.
| only_build_default_target: env("DOCSRS_ONLY_BUILD_DEFAULT_TARGET", false)?, | |
| include_default_targets: env("DOCSRS_INCLUDE_DEFAULT_TARGETS", true)?, |
And an example of DOCSRS_INCLUDE_DEFAULT_TARGETS=false in .env.sample seems like it'd support defaulting to on without the double-negative of !only_build_default_target.
There was a problem hiding this comment.
@Nemo157 well this is what I mean by I want the env variable to default to false - otherwise it's confusing what the default is. If we change it to INCLUDE_DEFAULT_TARGETS the default will be true.
There was a problem hiding this comment.
🤷 I don't find it that confusing when things default to on and you have to explicitly disable them in config, but I don't really care that much.
There was a problem hiding this comment.
Ok, I changed this to INCLUDE_DEFAULT_TARGETS, set it to be on by default in docs.rs, but off by default in env.sample.
This is still opt-in and off by default. However, it's useful for local development and custom registries.
jyn514
force-pushed
the
no-default-targets
branch
from
7e1fda3 to
7a073c4
Compare
This is still opt-in and off by default. However, it's useful for local
development and custom registries.
This is all of #532 except for actually switching it on. It does turn it on for local development, so that builds will be faster.
r? @pietroalbini
cc @l4l