path attribute on modules is useless in nested modules

[jethrogb]

My crate has some test support data in tests/support/mod.rs. When trying to load this module for unit tests like this:

mod tests {
    #[path="../tests/support/mod.rs"]
    mod support;
}

I get an error like this:

error: couldn't read "src/tests/../tests/support/mod.rs": No such file or directory (os error 2)

In fact, specifying pretty much any path is going to fail, because the src/tests/ directory does not exist, because I'm using a nested module and not a separate file. When using nested modules, I think the path at which relative path resolution should start should be the directory that the actual source file is in.

[Aatch]

@jethrogb #[path="../../tests/support/mod.rs"] should work.

I guess the issue is that both behaviours are reasonable, it depends on your mental model of how rustc finds module files. The thing is, the current behaviour is, well, what we currently have, meaning that changing it is technically a breaking change. It's also not obvious to me that "relative to the current file" is strictly better than what we have now, or just "different".

[jethrogb]

It's also not obvious to me that "relative to the current file" is strictly better than what we have now, or just "different".

Given that one option results in unresolvable paths, I'd say the other option is better. I think it could be non-breaking by testing whether the current way works, and if it doesn't work try the other way.

[jethrogb]

Another option is to let rustc, instead of the OS, resolve .., but doing path resolution in userspace is error-prone.

[jseyfried]

The following works for this use case:

#[path = "../tests"]
mod tests {
    #[path = "support/mod.rs"]
    mod support; // expects "../tests/support/mod.rs" (relative to source file directory)

    // --- or just ---

    mod support; // expects "../tests/support/mod.rs" or "../tests/support.rs"
}

If we were designing from scratch, I would probably prefer to have all paths be relative to the source file's directory.

This example is reasonable enough; the paths are relative to the current module's directory, i.e. the directory in which mod foo; would expect to find foo.rs, as opposed to the current file's directory.

However, when mod foo; isn't allowed (c.f. directory ownership restrictions), this definition doesn't make sense and the directory to which the path is relative feels arbitrary, a violation of the spirit of those restrictions.

That being said, I think the status quo here is acceptable enough that falling back to file-relative paths isn't worth the extra language complexity.

[jseyfried]

This issue is common enough that I think it would be worth checking the file-relative path before emitting the "no such file" error. If the path is file-relative, we would give an informative error message, ideally with an error code that can explains #[path] on inline modules (mod foo { ... }).

cc @GuillaumeGomez @jonathandturner

[GuillaumeGomez]

I'm not sure that adding a logic in such a case is a good idea. The error seems pretty explicit for me. However, we could still try to resolve the path instead of having this ".." in the middle.

[jseyfried]

@GuillaumeGomez

However, we could still try to resolve the path instead of having this ".." in the middle.

I'm wary of allowing more paths than we do today, but perhaps we could check if the user is trying to .. their way out of an inline module without a corresponding directory (like the example from https://github.com/rust-lang/rust/issues/35016#issue-167258874) and emit the specialized diagnostics then. The idea is to make #[path] on inline modules more discoverable here.

[GuillaumeGomez]

I'm feeling uneasy with having "hidden" logic in here. It's possible that the user wants the ./something/file.rs and that we deduce that it's some other path. It'd be quite a burden.

[GuillaumeGomez]

So after explanations, it's not about adding a new mechanism but clarifying the error. Therefore, I completely agree! 👍

[gnzlbg]

Any progress here? I am getting this error, and have been trying to work around it for 10 minutes after reading all the comments without any luck.

I don't understand how any workaround for this could even work. AFAICT the problem is that the relative path specified in #[path = ...] is appended to an invalid path, resulting in an invalid path that cannot be resolved. This happens because the path for nested modules contains a directory that does not exist.

Given two files:

• src/lib.rs
• src/bar.rs

and writing the following in lib.rs:

mod foo {
    #[path = "bar.rs"]
    mod bar;
}

will try to find bar.rs at src/foo/bar.rs. If I write

mod foo {
    #[path = "../bar.rs"]
    mod bar;
}

instead, rustc will try to find bar.rs at src/foo/../bar.rs.

Note how both paths contain src/foo in it, which is a directory that does not exist. That makes the path invalid (pointing to a non-existing file) in all architectures I've tried (MacOSX and Linux). Is there an X such that:

mod foo {
    #[path = "X/bar.rs"]
    mod bar;
}

will find bar.rs in the previously given directory structure?

If the answer is yes, then this is an error message bug, and the error message should suggest the appropriate path to use.

If the answer is no, this is either a bug in #[path], or a feature-request. Without this feature, #[path]'s doesn't appear to be usable in nested modules. Did the RFC for #[path] if there was ever one, consider this interaction? If not, how does the RFC for modules in Rust 2018 handle it?

[josephlr]

I've run into the problem @gnzlbg described in https://github.com/rust-lang/rust/issues/35016#issuecomment-409185342 both in once_cell and getrandom. A minimal implementation in rustc could just call Path::canonicalize before attempting to read the file. This would allow ../foo.rs to work as a #[path].

[koivunej]

Found this issue from a tail end of E-mentor issues, seems like something I could help solve. As far as I understand the situation is illustrated by the original issue description, a file ends up being opened at a non-canonicalized path showing an error like:

error: couldn't read src/tests/../tests/support/mod.rs: No such file or directory (os error 2)
 --> src/main.rs:7:5
  |
7 |     mod support;

• last comment by GuillaumeGomez had this should be a better error instead of fix to allow this
• call to Path::canonicalize was proposed in https://github.com/rust-lang/rust/issues/35016#issuecomment-615583878

I looked into introducing Path::canonicalize at the current implementation. I think that would always cause a readlink or equivalent which might have some TOCTOU issues. TOCTOU because it'd seem that there is currently only an fs::read_to_string call to load the file. The required canonicalization could be done manually at the callsite but the issue of complicating the overall logic is still there. "Done manually" as in pop elements from dir_path for each leading Component::ParentDir in rustc_expand::module::mod_file_path. Not sure if this could be a pandoras box leading to /etc/passwd inclusion somehow.

Regarding the "more original" better error fix the originally suggested hint of using two ../../ to escape the implicit subdirectory given for outer module (tests in the issue description). This doesn't at least (anymore?) work to escape it since https://github.com/rust-lang/rust/issues/44660. However, looking at the RFC#2126 directly it would seem that it didn't really mean to touch this part of the modules.

Another point raised in the error discussion about generally not being able to escape inline modules with #[path] so using one should just be an error. Currently this case is not covered in the reference, so it would have to be added there as well if not supported. Reference does explain #[path] in the context of outer inline module having #[path]. I'm just starting out the rustc journey, didn't yet look how I could create such "Inline modules cannot be escaped with upward relative #[path] inclusion" error.

Would like to work on getting this fixed, one way or the another but there's a decision to be made.

[koivunej]

Continued exploration on this.

It would seem that #[path = "/tmp/this_is_ok.rs"] works at the moment. So would the adding of error be to only give a better error to when escaping inline modules with all or non-navigable relative paths?

Path::canonicalize

This wouldn't work without removing the current behaviour of giving inline modules a directory path. As in the modified original example of:

// in src/lib.rs

mod this_is_inline {
  #[path = "../support.rs"]
  mod support;
}

Here a direct application of std::fs::canonicalize would end up trying to canonicalize src/this_is_inline/../support.rs, which in turn should fail if the src/this_is_inline didn't exist as a directory or symlink to directory (at least on Linux). The real "normalization" or "soft_canonicalization" fix would be to do what I already called "done manually at the callsite", which would allow:

• assert_eq!(soft_canonicalize("src/this_is_inline", "../support.rs"), "src/support.rs")
• continue supporting currently supported cases (absolute paths)
• avoid adding any syscalls before std::fs::read_to_string

[eminence]

Another wrinkle is that the current behavior (as of rust 1.65 today) is not consistent between platforms.

Given this code:

mod foo {
    #[path = "../bar.rs"]
    mod bar;
}

This will work on Windows, but not Linux

[nbaum]

If you're looking for a solution, this may suffice:

// In, say, mod.rs
#[path="."]
mod foo {
  #[path="."]
  mod bar {
    mod baz;
  }
}

This will load baz.rs from alongside mod.rs.