sagiegurari / cargo-make

Rust task runner and build tool.
https://sagiegurari.github.io/cargo-make/
Apache License 2.0
2.56k stars 124 forks source link

[Question]: Should `clear=true` on a task also clear the env of the task? #816

Closed Javagedes closed 1 year ago

Javagedes commented 1 year ago

Describe The Bug

In our current project, we override the pre-made common tasks (Build, Test, Coverage) with our own custom tasks, setting clear = true in the task as the documentation specifies that this deletes the existing task attributes. I personally read this as we are not extending the task, but completely overwriting it so only our task remains.

However, I'm experiencing an issue where my override of the coverage task fails when named coverage, but succeeds when named coverage2, hinting that some information from the old task remains.

Here is the task i'm referring to:

[tasks.coverage]
description = "Build and run all tests and calculate coverage."
clear = true
command = "cargo"
args = ["tarpaulin", "@@split(PACKAGE_TARGET, )", "--out", "Html", "--exclude-files", "**/tests/*", "--output-dir", "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target"]

I performed a cargo make --print-steps coverage to see if there was any difference, which there was:

when named coverage

env: Some(
  {
      "RUSTFLAGS": Value(
          "-C link-dead-code",
      ),
  },
),

when named coverage2

env: Some(
    {
        "CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE": Value(
            "C:\\src\\UefiRust\\Makefile.toml",
        ),
        "CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY": Value(
            "C:\\src\\UefiRust",
        ),
    },
),

I don't have an argument one way or the other if clear = true fully or partially overrides, but I just wanted to raise this issue to make sure that what I'm seeing is not a bug. If it is not a bug, I do think it could be documented a bit better as to what clear = true actually does.

Thanks for your time :)

sagiegurari commented 1 year ago

thanks @Javagedes for reporting. clear=true sshould remove everything. if its not, i'll fix. i'll check it out. CARGO_MAKE_CURRENTTASK... env vars are pushed automatically in runtime.

sagiegurari commented 1 year ago

@Javagedes sorry for the really really late reply. I found the issue and i hope the fix will resolve it. Can you please verify it in the 0.36.7 development branch

sagiegurari commented 1 year ago

cargo-make is now released with this fix. thanks for reporting and please try it out.