Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inputs tool logic and add new formatting options. #2485

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

digit-google
Copy link
Contributor

This fixes the logic used by the inputs tool to report correctly the inputs of phony targets.

  • Move the logic into its own InputsCollector class, and add proper unit-tests to it.

  • Add --no-shell-escape, --dependency-order and --print0 options to the tool to alter the order and format of the result for easier processing by tools.

  • Add new integration tests for the problematic cases.

Fixes #2482

@jhasse jhasse added this to the 1.13.0 milestone Aug 28, 2024
@mcprat
Copy link
Contributor

mcprat commented Aug 29, 2024

would it be too much to ask for this to be split into 3 commits?

Add a new class to wrap the logic needed to implement the
`inputs` tool correctly (see Issue ninja-build#2482 for details), and
provide a unit-test for it.
This uses the InputsCollector class introduced in the
previous patch to implement the tool properly. Results
are still shell-escaped and sorted alphabetically.

Fixed ninja-build#2482
Add new options to the `inputs` tool in order to change
the format of its output:

- `--no-shell-escape` to avoid shell-escaping the results.

- `--dependency-order` to return results in dependency order,
  instead of sorting them alphabetically.

- `--print0` to use \0 as the list separator in the list,
  useful to process the target paths with `xargs -0` and
  similar tools.
@digit-google
Copy link
Contributor Author

I have split the PR into three commits. I don't really need to --print0 option and can remove it for simplicity if you prefer.
@jhasse, can we submit this now? The correctness of this tool's output is very important for the Fuchsia build, as it is used after the build to pickup artifacts to upload to cloud storage, while filtering out stale files from previous incremental builds.

@jhasse jhasse merged commit 41ecb09 into ninja-build:master Sep 19, 2024
11 checks passed
@jhasse
Copy link
Collaborator

jhasse commented Sep 19, 2024

Thanks!

@digit-google digit-google deleted the fix-inputs-tool branch September 19, 2024 16:02
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.

Ninja -t inputs output is incorrect for phony targets
3 participants