Pitch: Native response file support

Related Projects ArgumentParser
1

Hello folks,

I have an draft PR that introduced native response files support to Swift Argument Parser Add native response file support by bkhouri · Pull Request #909 · apple/swift-argument-parser · GitHub

The PR allows for nested response files to be used, and as such, support for source location was added after a comment by @rauhul .

Here's what I currently have for source location information:

  1. command line argument validation error
    There is no change to the output If the command line does not contain any response files

    ❯ swift run swift-build --sbom-spec spdxss
[14 / 16] SwiftPMBuildServer

Build complete! (3.13 secs.)
error: The value 'spdxss' is invalid for '--sbom-spec <sbom-spec>'. Please provide one of 'cyclonedx', 'spdx', 'cyclonedx1' or 'spdx3'.
Help:  --sbom-spec <sbom-spec>  Set the SBOM specification and generate an SBOM.
Usage: swift build <options>
  See 'build -help' for more information.

    However, if the command line contains at least 1 response file, source information is added

    ❯ swift run swift-build --experimental-dump-arguments-source-location @test.response --sbom-spec spdx
[14 / 17] SwiftPMBuildServer

Build complete! (3.25 secs.)
error: The value 'swift-build' is invalid for '--build-system <build-system>'. Please provide one of 'native', 'swiftbuild' or 'Xcode'.
  at /Users/bkhouri/Documents/git/public/swiftlang/swift-package-manager/test.response:4
  included from argv[1]
  at /Users/bkhouri/Documents/git/public/swiftlang/swift-package-manager/test.response:5
  included from argv[1]
Help:  --build-system <build-system>  Specify the build system to use.
Usage: swift build <options>
  See 'build -help' for more information.

  2. Preview command line arguments source location
    There could possibly be a use case where the developer would like to know where all the arguments are populated. As such, a --experimental-dump-arguments-source-location argument can be supplied that will dump details about all the arguments and their location. This argument is defined in SAP, where external tool will get this automatically. By default, the argument outputs a human-readable option, and it supports =text or =json.

    Here's a sample output

    ❯ swift run swift-build --experimental-dump-arguments-source-location @test.response --sbom-spec spdx
[10 / 12] swift-crypto_CCryptoBoringSSL

Build complete! (3.04 secs.)
build
    ├── --package-path = nil   (default)
    ├── --cache-path = nil   (default)
    ├── --config-path = nil   (default)
    ├── --security-path = nil   (default)
<...SNIP...>
    ├── --explicit-target-dependency-import-check = none   (default)
    ├── --build-system = swiftbuild
          at /Users/bkhouri/Documents/git/public/swiftlang/swift-package-manager/test2.response:2
          included from /Users/bkhouri/Documents/git/public/swiftlang/swift-package-manager/test.response:4
          included from argv[1]
    ├── -debug-info-format = nil   (default)
    ├── --experimental-enable-codesize-profile = false   (default)
    ├── --experimental-codesize-profile-output-dir = nil   (default)
    ├── --sbom-spec = cyclonedx
          at argv[3]
    ├── --sbom-spec[1] = spdx
          at argv[3]
    ├── --sbom-output-dir = nil   (default)
    ├── --sbom-filter = nil   (default)
    └── --sbom-warning-only = false   (default)

At the time of writing, there is an issue in the PR related to tracking information when the option is an array.

Could I get some feedback on the source location information?

Thanks,

Sam

1 Like
2

Assuming the --experimental-dump-arguments-source-location is good enough to not be experimental, should this argument be opt-in or opt-out when running myCommand --help? what would the API be?

3

Overall, this helps to address my initial concern about how complex response file nesting can make finding where a particular argument/option/flag is set and also where the shadowing might be happening much easier.

Why the experimental prefix? It could be --dump-arguments-source-location instead. It's possibly unique enough that it wouldn't collide with any tools' flags. To be certain it could be prefixed with an SAP namespace.

4

I created this currently as experimental until I get feedback to ensure the behaviour is desirable, and is also accurate.

5

Thanks for working on this! The in-error message source location info looks great to me. I’d rather keep the API- or special flag-based access as a separate feature. We’ve had requests for that for a while, some of which would be used to implement bad patterns in CLI designs. Can we keep the source info internal at this time?

6

IMO a reasonable compromise is to include it in DEBUG builds at least initially (in a separate PR)

7

Works for me!

8

Would you be ok if we keep the special flag-based argument with the --experiemental prefix, indicating to the users this can change at any time? or would you prefer @rauhul suggestion of including it in DEBUG builds?

9

Thanks for the feedback folks.

I ended up splitting the XL PR in two M->L PRs:

The response file prefix defaults to @ and is customizable.

I thought about include some test in the --help output related to "this command supports response file", however i opted not to do this now for now as I would like get feedback on whether response files should be:

  1. Always available
  2. enabled by default
  3. disabled by default

Looking forward to more of your feedback.

1 Like
10

My gut feeling is that anything other than "disabled by default" would be untenable without a major version bump (if even then) since it would be a behavior-breaking change. It wouldn't be a source break but it's still a change to externally facing behavior, since an arg starting with @ today would just be parsed as a regular argument.

Compiler-brained folks like me might want to say "obviously it should be supported by default so that it's easy to use when we need it" but I don't know if that thinking extends out to general command line tools.

Would it make sense to change the responseFilePrefix requirement to be Character?, where nil (which would be the default) disables it, and a non-null Character is treated as an introducer?

11

I was thinking this when I sent this message asking for feedback :slight_smile: and you bring up a good point about potentially breaking existing behaviour where an arguments starts with @ today.

I'll update the PR shortly to make response file support opt-in.

1 Like