Skip to content

Use subdirectory within target/tests/#84

Merged
taiki-e merged 1 commit into
eupn:masterfrom
ijackson:own-target-dir
Mar 30, 2024
Merged

Use subdirectory within target/tests/#84
taiki-e merged 1 commit into
eupn:masterfrom
ijackson:own-target-dir

Conversation

@ijackson

@ijackson ijackson commented Jan 15, 2023

Copy link
Copy Markdown
Contributor

Use subdirectory within target/tests/

Or to put it another way, do not assume that the project's target/tests/target/ is ours to use as we wish.

Specifically,

  • Change the the pathname to pass to cargo in CARGO_TARGET_DIR to add / "macrotest" at the end.
  • Create it, which is now necessary, because it's not a parent of project.dir.
  • Construct it in Project::prepare, rather than in cargo(). (Because the creation should be done in prepare(), not cargo().)
  • So, pass the value though in a new pub field in Project.
  • Abolish the now-unused target_dir field of Project.

Empirically, this is sufficient on its own to resolve
#83
dtolnay/trybuild#218

(I don't know precisely what the difference is between trybuild's and macrotest's build invocations. It is possible that it would be better for cargo to be able to cache both in the target directory, but I think we should use a namespaced directory, regardless.)

I have submitted a similar merge request dtolnay/trybuild#219 for trybuild, which, likewise, is sufficient on its own to resolve the problem I was experiencing. I think these changes should be made to both packages.

Closes #83

Or to put it another way, do not assume that the project's
target/tests/target/ is ours to use as we wish.

Specifically,
  * Change the the pathname to pass to cargo in `CARGO_TARGET_DIR`
    to add `/ "macrotest"` at the end.
  * Create it, which is now necessary, because it's not a parent
    of `project.dir`.
  * Construct it in `Project::prepare`, rather than in `cargo()`.
    (Because the creation should be done in `prepare()`, not `cargo()`.)
  * So, pass the value though in a new `pub` field in `Project`.
  * Abolish the now-unused `target_dir` field of `Project`.

Empirically, this is sufficient on its own to resolve
  eupn#83
  dtolnay/trybuild#218

(I don't know precisely what the difference is between trybuild's and
mcarotest's build invocations.  It is possible that it would be better
for cargo to be able to cache both in the target directory, but I
think we should use a namespaced directory, regardless.)
@taiki-e taiki-e merged commit 739e0ef into eupn:master Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macrotest and trybuild together keep doing rebuilds

3 participants