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

make uninstall step based on inspecting install steps, rather than with "push installed file" mechanism #14943

Open
Tracked by #14647
andrewrk opened this issue Mar 16, 2023 · 1 comment
Open
Tracked by #14647

make uninstall step based on inspecting install steps, rather than with "push installed file" mechanism #14943

andrewrk opened this issue Mar 16, 2023 · 1 comment
Labels
bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. 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.

The way that zig build uninstall works is broken with regards to packages. It needs to be audited and to be changed to a different strategy than the pushInstalledFile system that is currently in place. Instead it should figure out the installed file paths based on inspecting the install steps that were created from running the build() function.
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Mar 16, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Mar 16, 2023
@andrewrk andrewrk mentioned this issue Mar 16, 2023
Merged
52 tasks
@rohlem
Copy link
Contributor

rohlem commented Mar 16, 2023 •
edited
Loading

IIUC, updating procedure would always be to uninstall the old version first, since the set of installed files might change due to the update.
I'm not sure if this is always a user-facing concern, or whether the package manager will have to care about this - maybe delegate to user if a to-be-updated package is detected to be installed?
(Can the build system detect whether a package is installed?
Also I assume that install may do more than just write files to a local zig-out directory - even though that's probably all my packages will ever do.)

Or on second thought maybe I'm conflating things, and install is supposed to be modular and play nice with nesting into other projects -> allow updates without uninstalling?
Then a separate system-install step could use custom logic for these things.

@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jul 20, 2023
andrewrk added a commit to ikskuh/zig that referenced this issue Jul 30, 2023
@andrewrk
build system: follow-up enhancements regarding LazyPath
810f3f5 
* introduce LazyPath.cwd_relative variant and use it for --zig-lib-dir. closes ziglang#12685
* move overrideZigLibDir and setMainPkgPath to options fields set once
  and then never mutated.
* avoid introducing Build/util.zig
* use doc comments for deprecation notices so that they show up in
  generated documentation.
* introduce InstallArtifact.Options, accept it as a parameter to
  addInstallArtifact, and move override_dest_dir into it. Instead of
  configuring the installation via Compile step, configure the
  installation via the InstallArtifact step. In retrospect this is
  obvious.
* remove calls to pushInstalledFile in InstallArtifact. See ziglang#14943
* rewrite InstallArtifact to not incorrectly observe whether a Compile
  step has any generated outputs. InstallArtifact is meant to trigger
  output generation.
* fix child process evaluation code handling of `-fno-emit-bin`.
* don't store out_h_filename, out_ll_filename, etc., pointlessly. these
  are all just simple extensions appended to the root name.
* make emit_directory optional. It's possible to have nothing outputted,
  for example, if you're just type-checking.
* avoid passing -femit-foo/-fno-emit-foo when it is the default
* rename ConfigHeader.getTemplate to getOutput
* deprecate addOptionArtifact
* update the random number seed of Options step caching.
* avoid using `inline for` pointlessly
* avoid using `override_Dest_dir` pointlessly
* avoid emitting an executable pointlessly in test cases
* fix compiler_rt_panic test case warning about _start

Removes forceBuild and forceEmit. Let's consider these additions separately.
Nearly all of the usage sites were suspicious.
andrewrk added a commit to ikskuh/zig that referenced this issue Jul 30, 2023
@andrewrk
build system: follow-up enhancements regarding LazyPath
38840e2 
* introduce LazyPath.cwd_relative variant and use it for --zig-lib-dir. closes ziglang#12685
* move overrideZigLibDir and setMainPkgPath to options fields set once
  and then never mutated.
* avoid introducing Build/util.zig
* use doc comments for deprecation notices so that they show up in
  generated documentation.
* introduce InstallArtifact.Options, accept it as a parameter to
  addInstallArtifact, and move override_dest_dir into it. Instead of
  configuring the installation via Compile step, configure the
  installation via the InstallArtifact step. In retrospect this is
  obvious.
* remove calls to pushInstalledFile in InstallArtifact. See ziglang#14943
* rewrite InstallArtifact to not incorrectly observe whether a Compile
  step has any generated outputs. InstallArtifact is meant to trigger
  output generation.
* fix child process evaluation code handling of `-fno-emit-bin`.
* don't store out_h_filename, out_ll_filename, etc., pointlessly. these
  are all just simple extensions appended to the root name.
* make emit_directory optional. It's possible to have nothing outputted,
  for example, if you're just type-checking.
* avoid passing -femit-foo/-fno-emit-foo when it is the default
* rename ConfigHeader.getTemplate to getOutput
* deprecate addOptionArtifact
* update the random number seed of Options step caching.
* avoid using `inline for` pointlessly
* avoid using `override_Dest_dir` pointlessly
* avoid emitting an executable pointlessly in test cases

Removes forceBuild and forceEmit. Let's consider these additions separately.
Nearly all of the usage sites were suspicious.
QusaiHroub pushed a commit to QusaiHroub/zig that referenced this issue Aug 3, 2023
@andrewrk @QusaiHroub
build system: follow-up enhancements regarding LazyPath
bedca92 
* introduce LazyPath.cwd_relative variant and use it for --zig-lib-dir. closes ziglang#12685
* move overrideZigLibDir and setMainPkgPath to options fields set once
  and then never mutated.
* avoid introducing Build/util.zig
* use doc comments for deprecation notices so that they show up in
  generated documentation.
* introduce InstallArtifact.Options, accept it as a parameter to
  addInstallArtifact, and move override_dest_dir into it. Instead of
  configuring the installation via Compile step, configure the
  installation via the InstallArtifact step. In retrospect this is
  obvious.
* remove calls to pushInstalledFile in InstallArtifact. See ziglang#14943
* rewrite InstallArtifact to not incorrectly observe whether a Compile
  step has any generated outputs. InstallArtifact is meant to trigger
  output generation.
* fix child process evaluation code handling of `-fno-emit-bin`.
* don't store out_h_filename, out_ll_filename, etc., pointlessly. these
  are all just simple extensions appended to the root name.
* make emit_directory optional. It's possible to have nothing outputted,
  for example, if you're just type-checking.
* avoid passing -femit-foo/-fno-emit-foo when it is the default
* rename ConfigHeader.getTemplate to getOutput
* deprecate addOptionArtifact
* update the random number seed of Options step caching.
* avoid using `inline for` pointlessly
* avoid using `override_Dest_dir` pointlessly
* avoid emitting an executable pointlessly in test cases

Removes forceBuild and forceEmit. Let's consider these additions separately.
Nearly all of the usage sites were suspicious.
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Aug 5, 2023
@andrewrk andrewrk modified the milestones: 0.12.0, 0.13.0 Feb 14, 2024
@Vexu Vexu mentioned this issue Mar 30, 2024
Closed
@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Jul 5, 2024
andrewrk added a commit that referenced this issue Jul 9, 2024
@andrewrk
std.Build: remove the "push installed file" mechanism
99fac50 
Tracked by #14943
andrewrk added a commit that referenced this issue Jul 10, 2024
@andrewrk
std.Build: remove the "push installed file" mechanism
d6407d9 
Tracked by #14943
andrewrk added a commit that referenced this issue Jul 12, 2024
@andrewrk
std.Build: remove the "push installed file" mechanism
26d506c 
Tracked by #14943
eric-saintetienne pushed a commit to eric-saintetienne/zig that referenced this issue Jul 16, 2024
@andrewrk @eric-saintetienne
std.Build: remove the "push installed file" mechanism
6f0e836 
Tracked by ziglang#14943
SammyJames pushed a commit to SammyJames/zig that referenced this issue Aug 7, 2024
@andrewrk @SammyJames
std.Build: remove the "push installed file" mechanism
719e7a5 
Tracked by ziglang#14943
igor84 pushed a commit to igor84/zig that referenced this issue Aug 11, 2024
@andrewrk
std.Build: remove the "push installed file" mechanism
cac3ec8 
Tracked by ziglang#14943
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Milestone
0.15.0
Development

No branches or pull requests
2 participants
@andrewrk @rohlem