bugfix: make spack pkg grep respect windows CLI limits#50520
Merged
bugfix: make spack pkg grep respect windows CLI limits#50520
spack pkg grep respect windows CLI limits#50520Conversation
3d7766e to
aeaa2bb
Compare
42c1396 to
8048101
Compare
`spack pkg grep` can construct command lines that are too long for Windows, i.e. command lines that are longer than 32768 characters. This makes `spack pkg grep` respect the Windows limit by default, and gets the unix limit from `sysconfig`. - [x] Add a new `spack.cmd.group_arguments` function to create CLI-safe arg groups - [x] Default to max 500 elements or 32768 chars, whichever comes first - [x] If sysconfig is available, get `SC_ARG_MAX` and use that for max chars - [x] Clean up output handling in `test_pkg_grep` test - [x] Add test for `group_arguments` Signed-off-by: Todd Gamblin <[email protected]>
8048101 to
be57175
Compare
Spack's `Executable` class isn't properly returning a called process's return code when it fails with a `CalledProcessError`. Record it before raising a `ProcessError` so that client code can query it later. Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Signed-off-by: Todd Gamblin <[email protected]>
Contributor
|
@tgamblin Would you like me to push up some of the executable script tests for the windows side? |
johnwparent
approved these changes
Jun 2, 2025
Contributor
johnwparent
left a comment
There was a problem hiding this comment.
Looks good, nothing to hold up a merge.
Member
Author
|
@johnwparent thanks! can yo duo that as a follow-on? |
Contributor
Absolutely, LGTM |
kshea21
pushed a commit
to kshea21/spack
that referenced
this pull request
Jun 18, 2025
`spack pkg grep` can construct command lines that are too long for Windows, i.e. command lines that are longer than 32768 characters. This makes `spack pkg grep` respect the Windows limit by default, and gets the unix limit from `sysconfig`. - [x] Add a new `spack.cmd.group_arguments` function to create CLI-safe arg groups - [x] Default to max 500 elements or 32768 chars, whichever comes first - [x] If sysconfig is available, get `SC_ARG_MAX` and use that for max chars - [x] Make `Executable` always return an error if it's not successful - [x] Add test for `group_arguments` --------- Signed-off-by: Todd Gamblin <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
spack pkg grepcan construct command lines that are too long for Windows, i.e. command lines that are longer than 32768 characters.This makes
spack pkg greprespect the Windows limit by default, and gets the unix limit fromsysconfig.spack.cmd.group_argumentsfunction to create CLI-safe arg groupsSC_ARG_MAXand use that for max charsgroup_arguments