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.

Sign up for GitHub

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

Jump to bottom

rename --verbose to --verbose-spawn and limit the scope of what it does #14970

Open
Tracked by #14647
andrewrk opened this issue Mar 17, 2023 · 0 comments
Open
Tracked by #14647

rename --verbose to --verbose-spawn and limit the scope of what it does #14970

andrewrk opened this issue Mar 17, 2023 · 0 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone
0.15.0

Comments

@andrewrk
Copy link
Member

Extracted from #14647.

Redefine it to: print to stderr the commandline before spawning any child process.

Audit all uses of the verbose flag and all child process invocations.

zig/lib/build_runner.zig

Lines 117 to 118 in 68c7261

if (mem.eql(u8, arg, "--verbose")) {
builder.verbose = true;

zig/lib/build_runner.zig

Line 946 in 68c7261

\\ --verbose Print commands before executing them

@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Mar 17, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Mar 17, 2023
@andrewrk andrewrk mentioned this issue Mar 17, 2023
Merged
52 tasks
tjog added a commit to tjog/zig that referenced this issue Jul 22, 2023
@tjog
build: split --verbose to specific flags
bf78c60 
Usage (excluding initialization) of `build.verbose` appears in two
files:

1. lib/std/Build.zig
2. lib/std/Build/Step.zig

As it turns out only Step.zig is relevant in the new context of
`--verbose-spawn`. The verbose references in Build.zig are through
`makeUninstall` and `truncateFile`. The first of which is a bunch of
deletion operations, and the second of which only got called from
InstallDir.zig.

The `truncateFile` operation in itself is general enough to not depend
on a specific flag and the log call is therefore hoisted to the
InstallDir.zig callsite. The logging in InstallDir is then enabled by
a new parameter `--verbose-install`. I also took the liberty of adding
logging to the result of updateFile operations as well under this flag.
The updateFile logic is also used in InstallFile and InstallArtifact, so
the helper `handleVerboseInstallUpdateFile` was created and added to all
call site results.

Regarding audit of child process invocations in Build.zig and default
Steps:

`execAllowFail` is the child process spawning part of the Build API that
did not log invocations, so a call to `handleVerboseSpawn2` was added.

The other APIs are `evalZigProcess` and `evalChildProcess`, but they
already use the `handleVerboseSpawn` helper.

Closes ziglang#14970
tjog added a commit to tjog/zig that referenced this issue Jul 22, 2023
@tjog
build: split --verbose to specific flags
d2c6c21 
Usage (excluding initialization) of `build.verbose` appears in two
files:

1. lib/std/Build.zig
2. lib/std/Build/Step.zig

As it turns out only Step.zig is relevant in the new context of
`--verbose-spawn`. The verbose references in Build.zig are through
`makeUninstall` and `truncateFile`. The first of which is a bunch of
deletion operations, and the second of which only got called from
InstallDir.zig.

The `truncateFile` operation in itself is general enough to not depend
on a specific flag and the log call is therefore hoisted to the
InstallDir.zig callsite. The logging in InstallDir is then enabled by
a new parameter `--verbose-install`. I also took the liberty of adding
logging to the result of updateFile operations as well under this flag.
The updateFile logic is also used in InstallFile and InstallArtifact, so
the helper `handleVerboseInstallUpdateFile` was created and added to all
call site results.

Regarding audit of child process invocations in Build.zig and default
Steps:

`execAllowFail` is the child process spawning part of the Build API that
did not log invocations, so a call to `handleVerboseSpawn2` was added.

The other APIs are `evalZigProcess` and `evalChildProcess`, but they
already use the `handleVerboseSpawn` helper.

Closes ziglang#14970
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jul 22, 2023
tjog added a commit to tjog/zig that referenced this issue Jul 23, 2023
@tjog
build: split --verbose to specific flags
0fa52e0 
Usage (excluding initialization) of `build.verbose` appears in two
files:

1. lib/std/Build.zig
2. lib/std/Build/Step.zig

As it turns out only Step.zig is relevant in the new context of
`--verbose-spawn`. The verbose references in Build.zig are through
`makeUninstall` and `truncateFile`. The first of which is a bunch of
deletion operations, and the second of which only got called from
InstallDir.zig.

The `truncateFile` operation in itself is general enough to not depend
on a specific flag and the log call is therefore hoisted to the
InstallDir.zig callsite. The logging in InstallDir is then enabled by
a new parameter `--verbose-install`. I also took the liberty of adding
logging to the result of updateFile operations as well under this flag.
The updateFile logic is also used in InstallFile and InstallArtifact, so
the helper `handleVerboseInstallUpdateFile` was created and added to all
call site results.

Regarding audit of child process invocations in Build.zig and default
Steps:

`execAllowFail` is the child process spawning part of the Build API that
did not log invocations, so a call to `handleVerboseSpawn2` was added.

The other APIs are `evalZigProcess` and `evalChildProcess`, but they
already use the `handleVerboseSpawn` helper.

Closes ziglang#14970
@tjog tjog mentioned this issue Jul 23, 2023
Closed
tjog added a commit to tjog/zig that referenced this issue Jul 23, 2023
@tjog
build: split --verbose to specific flags
b186999 
Usage (excluding initialization) of `build.verbose` appears in two
files:

1. lib/std/Build.zig
2. lib/std/Build/Step.zig

As it turns out only Step.zig is relevant in the new context of
`--verbose-spawn`. The verbose references in Build.zig are through
`makeUninstall` and `truncateFile`. The first of which is a bunch of
deletion operations, and the second of which only got called from
InstallDir.zig.

The `truncateFile` operation in itself is general enough to not depend
on a specific flag and the log call is therefore hoisted to the
InstallDir.zig callsite. The logging in InstallDir is then enabled by
a new parameter `--verbose-install`. I also took the liberty of adding
logging to the result of updateFile operations as well under this flag.
The updateFile logic is also used in InstallFile and InstallArtifact, so
the helper `handleVerboseInstallUpdateFile` was created and added to all
call site results.

Regarding audit of child process invocations in Build.zig and default
Steps:

`execAllowFail` is the child process spawning part of the Build API that
did not log invocations, so a call to `handleVerboseSpawn2` was added.

The other APIs are `evalZigProcess` and `evalChildProcess`, but they
already use the `handleVerboseSpawn` helper.

Closes ziglang#14970
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Aug 5, 2023
tjog added a commit to tjog/zig that referenced this issue Oct 11, 2023
@tjog
build: split --verbose to specific flags
22c65b4 
Usage (excluding initialization) of `build.verbose` appears in two
files:

1. lib/std/Build.zig
2. lib/std/Build/Step.zig

As it turns out only Step.zig is relevant in the new context of
`--verbose-spawn`. The verbose references in Build.zig are through
`makeUninstall` and `truncateFile`. The first of which is a bunch of
deletion operations, and the second of which only got called from
InstallDir.zig.

The `truncateFile` operation in itself is general enough to not depend
on a specific flag and the log call is therefore hoisted to the
InstallDir.zig callsite. The logging in InstallDir is then enabled by
a new parameter `--verbose-install`. I also took the liberty of adding
logging to the result of updateFile operations as well under this flag.
The updateFile logic is also used in InstallFile and InstallArtifact, so
the helper `handleVerboseInstallUpdateFile` was created and added to all
call site results.

Regarding audit of child process invocations in Build.zig and default
Steps:

`execAllowFail` is the child process spawning part of the Build API that
did not log invocations, so a call to `handleVerboseSpawn2` was added.

The other APIs are `evalZigProcess` and `evalChildProcess`, but they
already use the `handleVerboseSpawn` helper.

Closes ziglang#14970
@andrewrk andrewrk modified the milestones: 0.12.0, 0.13.0 Mar 21, 2024
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Milestone
0.15.0
Development

Successfully merging a pull request may close this issue.

build: split --verbose to specific flags tjog/zig
1 participant
@andrewrk