cargo package doesn't include build script if this has a custom path

[cdecompilador]

Problem

One package can have an entry on the manifest indicating a custom path for the build script, for example:

[package]
build = "../folder/build.rs"

Steps

Create a multiple packages that share a single build script and for programmability reasons keep just a single build script

Possible Solution(s)

The expected behaviour in this case would be to ship that build script on the root, and a naive solution would be to modify the behavior of the build attribute to be optional, so in case that the path is not found, default to the root.

The perfect solution would be to modify that entry also on the package, but I don't know if modification of the manifest is on the philosophy of cargo package

I put the example of ../folder/build.rs because is the worst-case scenario in which the naive solution is clearly naive, since that dir might exist with another context.

Notes

No response

Version

cargo 1.65.0 (4bc8f24d3 2022-10-20)
release: 1.65.0
commit-hash: 4bc8f24d3e899462e43621aab981f6383a370365
commit-date: 2022-10-20
host: x86_64-pc-windows-msvc
libgit2: 1.5.0 (sys:0.15.0 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:Schannel)
os: Windows 10.0.19044 (Windows 10 Enterprise LTSC 2021) [64-bit]

[weihanglo]

Oh this doesn't look nice. Thank you for reporting this!

If you have no time to wait for a fix but still want to reuse the shared build script, a workaround would be: Inside your package, create a symlink to the shared file. Then modify the value package.build to the symlink. cargo package generally follows symlinks and copies file over, so build script should be included automatically.

Besides, I believe cargo package without --no-verify will fail as Cargo cannot find build script.

[weihanglo]

Instructions to fix this

❗ This overlooked macros like include_str! resolving from the path relative to the file.
See #11383 (comment).
A new instructions is here #11383 (comment)

I noticed that package.readme and package.license-file fall into the same trap. Thankfully, they've got addressed, and the solution is pretty close to what @cdecompilador suggested: modifying the manifest file during packaging.

So, if anyone wants to help with this, here are some rough instructions in my mind:

1. When Cargo creates a list of archive files, it takes extra processes on readme and license-file here. We could do the same thing on build.
2. Consider refactor this piece of code into check_for_file_and_add as there will be 3 fields under a similar process.
3. cargo package normalizes your Cargo.toml and include the normalized one in the .crate file. It copies readme and license-file outside the package to package root. We may reuse the logic here for package.build.
4. Under tests/testsuite/package.rs, search license-file and you will find a bunch of tests there. I'd recommend finding a way to share the construction or even the assertion path of those tests.

Use @rustbot claim if you want to work on it. Don't be shy to ask questions here or on Zulip.

[cdecompilador]

@rustbot claim

[ehuss]

I don't think this is feasible to fix in the same way as readme or license-file. Although it can possibly copy the root file, cargo wouldn't be able to know about modules (or any other resources included via macros), thus not copying the entire crate. This issue also applies to the path key of target configurations. I think it would be reasonable to detect this situation and present an error when packaging.

[weihanglo]

Oh deer. You're absolutely right, @ehuss. I totally forgot there is a thing called macro.

Here are possible steps of the strategy Eric suggested:

1. In build_ar_list, check if each targets contains a path inside package root (targets can be accessed via pkg.manifest().targets()).
2. If it is outside the package, Cargo then gives users an error. The message could be:

   the source file of {target_kind} target "{name}" doesn't appear to be a path inside of the package.
   
   It is at {path}, whereas the root the package is {pkg_root}. This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files.
   
   Consider updating the path of {target_kind} target "{target_name}"  to point to a path inside the root of the package to resolve this error.
   

   (Please polish it a bit. Build script might need a customized message.)
3. Extract them to a function if needed. You may also want to collect all errors and present them all at once.

[cdecompilador]

hmmm, this limits a bit the possible usage of the build = ... attribute in the manifest, but what @ehuss pointed is a big concern, I will try fix it like @weihanglo said but this makes build = ... less useful, since for example in the repo I wanted to use it was needed to avoid having to maintain a multiple build.rs that had to be the same for multiple crates, so instead of using symlinks (which are a pain on windows) I suggested using the build = ... attribute.

Another harder solution and in my opinion logical is that no matter the path of the build.rs its always executed as if it was on the root of the package that calls it, but this may be a breaking change, also this change fixes the issue with adding modules to the build.rs

[VincentWo]

Hey @cdecompilador are you still working on this?
Everything's fine if you are, but I could also take over.

[cdecompilador]

I don't know why but the commit from where I started didn't pass half the tests, even some unrelated to my project, I will reset and try again so don't worry, also the code was almost done so it shouldn't be much work

[weihanglo]

Release assignment due to inactivity. Feel free to pick it up again.

@rustbot release-assignment

[yerke]

@rustbot claim

[linyihai]

There has also been no progress on this issue recently, and I have time to contribute to this issue.

[epage]

@rustbot release-assignment

[epage]

@linyihai if you weren't aware, we have office hours for contributors to talk over their work synchronously with members of the cargo ream.

[linyihai]

@linyihai if you weren't aware, we have office hours for contributors to talk over their work synchronously with members of the cargo ream.

I've heard of office hours, but I haven't done it yet. I'll have a chance to do it next time.

[linyihai]

There has also been no progress on this issue recently, and I have time to contribute to this issue.

@rustbot claim

Having understood the context of the problem, I intend to look at the problem. Some of the work has already been done , and I intend to build on it.