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

path arguments to zig build are inconsistent in whether they are relative to build root, such as --zig-lib-dir #12685

Closed
andrewrk opened this issue Aug 30, 2022 · 0 comments
Labels
bug Observed behavior contradicts documented or intended behavior zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

Here we collect zig lib dir from the user:

zig/src/main.zig

Lines 3769 to 3770 in 75d5a4b

override_lib_dir = args[i];
try child_argv.appendSlice(&[_][]const u8{ arg, args[i] });

It is then used relative to the cwd in order to build the build runner. Next, it is passed to the build runner and then the build runner interprets the file path as being relative to build.zig.

It can't be both ways; it needs to be consistently one way.

This results in nonsense experiences such as this:

$ stage3/bin/zig build  --zig-lib-dir ../lib
error: unable to open zig lib directory from 'zig-lib-dir' argument or env, '/home/andy/dev/lib': FileNotFound
$ stage3/bin/zig build  --zig-lib-dir lib
error: unable to open zig lib directory from 'zig-lib-dir' argument: 'lib': FileNotFound

As a workaround until this issue is fixed an absolute path can be used:

$ stage3/bin/zig build  --zig-lib-dir $(pwd)/../lib
(ok)
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior zig build system std.Build, the build runner, `zig build` subcommand, package management labels Aug 30, 2022
@andrewrk andrewrk added this to the 0.10.0 milestone Aug 30, 2022
andrewrk added a commit that referenced this issue Aug 30, 2022
@andrewrk andrewrk modified the milestones: 0.10.0, 0.10.1 Oct 12, 2022
andrewrk added a commit that referenced this issue Dec 7, 2022
andrewrk added a commit that referenced this issue Dec 7, 2022
ikskuh pushed a commit to ikskuh/zig that referenced this issue Jan 5, 2023
The problem was that paths in the cache are assumed to endwithout the path separator, but are added verbatim from the command line
in several places. The solution here is to make sure that all prefixes are without path separator by cutting it off when it is passed
to Cache.addPrefix. This way, the assumption is met.
ikskuh pushed a commit to ikskuh/zig that referenced this issue Jan 5, 2023
The problem was that paths in the cache are assumed to endwithout the path separator, but are added verbatim from the command line
in several places. The solution here is to make sure that all prefixes are without path separator by cutting it off when it is passed
to Cache.addPrefix. This way, the assumption is met.
@andrewrk andrewrk modified the milestones: 0.10.1, 0.11.0 Jan 10, 2023
andrewrk pushed a commit to ikskuh/zig that referenced this issue Jan 27, 2023
The problem was that paths in the cache are assumed to endwithout the path separator, but are added verbatim from the command line
in several places. The solution here is to make sure that all prefixes are without path separator by cutting it off when it is passed
to Cache.addPrefix. This way, the assumption is met.
@andrewrk andrewrk modified the milestones: 0.11.0, 0.11.1 Jul 24, 2023
andrewrk added a commit to ikskuh/zig that referenced this issue Jul 30, 2023
* 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 andrewrk modified the milestones: 0.11.1, 0.11.0 Jul 31, 2023
QusaiHroub pushed a commit to QusaiHroub/zig that referenced this issue Aug 3, 2023
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant