Tracking Issue for proc_macro::{tracked_env, tracked_path}

[m-ou-se]

View all comments

Feature gate: #![feature(proc_macro_tracked_env, proc_macro_tracked_path)]

This is a tracking issue for proc_macro::tracked*, to allow adding files and environment variables to the build system's dependency tracking.

Public API

// proc_macro

mod tracked {
    pub fn env_var<K: AsRef<OsStr> + AsRef<str>>(key: K) -> > Result<String, VarError>;
    pub fn path<P: AsRef<Path>>(path: P);
}

Steps / History

• Implementation of tracked_env::var: proc_macro: Add API for tracked access to environment variables #74653
• Implementation of tracked_path::path: add track_path::path fn for usage in proc_macros #84029
• Add tracking issue: Use new tracking issue for proc_macro::tracked_*. #99516
• Change path to accept AsRef<Path> instead of AsRef<str>
• Design the public API
• Documentation
• Final comment period (FCP)
• Stabilization PR

Unresolved Questions

• The full API design.
  • The implementaiton PRs were mostly focussed on the implementation details behind the scenes of proc_macro, but there's barely been any discussion on the public interface.

[m-ou-se]

This feature is not in a great state. The original PRs were mostly about the implementation details, with almost no discussion on the public API. There's still many unanswered design questions, such as naming, whether path should return something (a File? a String? a TokenStream? a PathBuf? nothing?), how it should be documented, how these functions should relate to each other, whether they should report errors through panics or through Result, whether it should take AsRef<OsStr> or AsRef<str>, and so on.

Instead of discussing every single detail separately, it'd probably be best to start with a more complete proposal for how these APIs should look, and then discuss that.

(Usually most of this happens before an unstable feature is merged and tracked, but this has already been merged, hence this tracking issue.)

[mitsuhiko]

Considering these APIs effectively mirror the cargo:rerun-if-changed and cargo:rerun-if-env-changed functionality from build scripts I think it's reasonable to assume they don't return anything but just register interest.

Right now the main reason you run into this issue is because someone already has functionality in a build script and wants to convert it into a proc_macro. I would also strongly recommend for it to not report an error if the file does not exist as absence and presence of a file is both legitimate for proc macros the same way as it works for build scripts again.

[bjorn3]

Considering these APIs effectively mirror the cargo:rerun-if-changed and cargo:rerun-if-env-changed functionality from build scripts I think it's reasonable to assume they don't return anything but just register interest.

I don't think that is a good argument. It is relatively common to forget to register interest with build scripts I believe. In addition it is pretty common to write helper functions that register interest and at the same time read the file/env var to prevent forgetting this. The reason that no such function exists by default for build scripts is because there is no way for cargo to provide a crate to build scripts with said functions. Furthermore returning the value is essential for making proc macros work with virtual filesystems and with compiler session local env vars.

[mitsuhiko]

It is relatively common to forget to register interest with build scripts I believe

Maybe? But that is probably a very different argument to begin with. Today where I'm standing there is functionality available with build.rs that you cannot have with proc_macro and the same arguments for the use of it with build.rs are equally appying to proc macros.

In addition it is pretty common to write helper functions that register interest and at the same time read the file/env var to prevent forgetting this.

By coupling these things together this has a high change of turning into a never ending argument because there will always be a disagreement about how this should work. In fact, even today you will find no agreement on how to best use the build script equivalents and if we had to come to an agreement about his this should work there, we probably wouldn't have solution there today with build scripts.

If there is a need for such an API it can always be added at a later point.

The reason that no such function exists by default for build scripts is because there is no way for cargo to provide a crate to build scripts with said functions.

Of course there is. It would always have been possible for cargo to ship a utility library that is linked against build scripts the same way as proc_macro exposes itself.

Furthermore returning the value is essential for making proc macros work with virtual filesystems and with compiler session local env vars.

That same argument can be brought up for build scripts.

[bjorn3]

If there is a need for such an API it can always be added at a later point.

The interest registering version can be trivially implemented in terms of the version that returns the value by discarding the return value, but not the other way around. In addition the interest registering versions may become entirely useless if we start sandboxing proc macros depending on the way we do this.

That same argument can be brought up for build scripts.

For build scripts it isn't all that important that changing a file in an editor without saving it reruns them. It is essential for proc macros. In addition rust-analyzer is already having a lot of trouble with proc macros going outside of their implied (unenforced) sandbox, while it doesn't have any issues with build scripts as those run in the same environment as with a regular cargo build due to rust-analyzer using cargo for invoking them.

[mitsuhiko]

I want to leave another piece of input here that I believe are relevant: rerun-if-changed today can track a directory and will in fact track changes to the contents. My expectation as a user would be that I can replicate the same functionality for proc_macros. This is in fact right now what I care about was I'm trying to build something that bundles folders.

The interest registering version can be trivially implemented in terms of the version that returns the value by discarding the return value, but not the other way around.

If returning contents turns out to be useful it can always be added as an additional API. I think coupling these things will make this feature stay in not stable land for a lot longer than if those discussions are separated.

[est31]

I think tracked_path::path should be redesigned to also open read access to the specified path, as suggested by RFC 3200, because the current API leaves that question unresolved, which basically begs for the proc macro to use outside methods to access files, which would be a break of current policy that this is unsupported.