Skip to content

std/build: change default install prefix to zig-out - #8638

Merged
andrewrk merged 2 commits into
ziglang:masterfrom
ifreund:build-default-prefix
Apr 30, 2021
Merged

std/build: change default install prefix to zig-out #8638
andrewrk merged 2 commits into
ziglang:masterfrom
ifreund:build-default-prefix

Conversation

@ifreund

Copy link
Copy Markdown
Member

Closes#7296

@ifreundifreund added zig build system std.Build, the build runner, `zig build` subcommand, package management breaking Implementing this issue could cause existing code to no longer compile or have different behavior. labels Apr 29, 2021
@ifreund

ifreund commented Apr 29, 2021

Copy link
Copy Markdown
MemberAuthor

This turns out to be awkward for the zig repository itself as a plain zig build in the root of the repo now creates ./bin/zig (totally OK) and ./lib/zig/** (a little awkward, as the other children of ./lib are tracked by git).

We can certainly add these paths to the .gitignore before merge, but I'd rather have a more clear division between which files in ./lib are version controlled and which are the result of a build.

@andrewrk

Copy link
Copy Markdown
Member

Hmm yeah let's reconsider this in light of this new observation. I think we didn't take it into account when making the decision, and it could very well be an issue with any project in general.

@ifreund

Copy link
Copy Markdown
MemberAuthor

Hmm yeah let's reconsider this in light of this new observation. I think we didn't take it into account when making the decision, and it could very well be an issue with any project in general.

One solution to this (which is starting to grow on me) is to require-p/--prefix in all cases. This makes it abundantly clear to the user where the artifacts will be placed, simplifies the behavior of the zig build system, and dodges this awkwardness.

For the zig repository, we already have build/ and build-* ignored so user could simply use those directories as their prefix.

@marler8997

Copy link
Copy Markdown
Contributor

Much like tar files, I think if install creates more than one entry it can be "annoying". I would be fine if the default was just to put everything in a single subdirectory, named "install" or something.

@Jarred-Sumner

Jarred-Sumner commented Apr 29, 2021

Copy link
Copy Markdown
Contributor

Nice. zig-cache was pretty confusing to me initially

One solution to this (which is starting to grow on me) is to require -p/--prefix in all cases. This makes it abundantly clear to the user where the artifacts will be placed, simplifies the behavior of the zig build system, and dodges this awkwardness.

Hmm yeah let's reconsider this in light of this new observation. I think we didn't take it into account when making the decision, and it could very well be an issue with any project in general.

What about overridable conventions for build types?

  • zig build-exe => bin/project-name
  • zig build-lib => lib/project-name
  • zig build-obj => include/project-name (not really sure about this one)

For backwards compatibility, this string could be inlined into build.zig by default on zit init-exe or zig init-lib going forward -- that way, no projects break unexpectedly.

I think having a default is worthwhile because it will let people install software written in Zig without having to think as much ("which folder does this project build to?")

Edit: I did not read the linked issue closely enough

@jmc-88

Copy link
Copy Markdown
Contributor

My unsolicited 2¢...

This turns out to be awkward for the zig repository itself as a plain zig build in the root of the repo now creates ./bin/zig (totally OK) and ./lib/zig/** (a little awkward, as the other children of ./lib are tracked by git).

We can certainly add these paths to the .gitignore before merge, but I'd rather have a more clear division between which files in ./lib are version controlled and which are the result of a build.

This is certainly going to cause issues to any project handled by a form of VCS. Keeping artifacts contained to zig-cache sounds better to me.

Requiring -p/--prefix would work very nicely if there was a separate zig install step, but if everyzig build invocation requires an explicit prefix it quickly becomes tedious for the user.

@g-w1

g-w1 commented Apr 29, 2021

Copy link
Copy Markdown
Contributor

I was imagining that it would be like CMAKE where you have to run it in the build directory, or use -p build. (s/build/anything_else/) I'm not sure how this is a problem. I find it pretty easy to just remember to use cmake from build.

@jmc-88

Copy link
Copy Markdown
Contributor

I was imagining that it would be like CMAKE where you have to run it in the build directory, or use -p build. (s/build/anything_else/) I'm not sure how this is a problem. I find it pretty easy to just remember to use cmake from build.

I don't think the suggestion was to generate a (potentially out-of-tree) build directory and do things from there, but rather to invoke zig build from the directory containing build.zig, and pass an explicit -p/--prefix pointing to the location where build artifacts should be installed.

What you're suggesting can instead already be done with something like:

$ mkdir build
$ cd build
$ zig build --build-file ../build.zig

@g-w1

g-w1 commented Apr 29, 2021

Copy link
Copy Markdown
Contributor

You don't need build file, it searches upwards. Either of these two options is satisfactory for me:

zig build -p build # makes build directory
mkdir build; cd build; zig build

@Jarred-Sumner

Copy link
Copy Markdown
Contributor

I was imagining that it would be like CMAKE where you have to run it in the build directory, or use -p build. (s/build/anything_else/) I'm not sure how this is a problem. I find it pretty easy to just remember to use cmake from build.

imo, depends on the target audience. Zig developers? It's fine to learn that.

But a random developer installing a CLI utility they saw on Hacker News? If it doesn't install successfully on the first try, they might not bother. Every barrier means a lower conversion rate. Can't expect developers to always write build instructions and can't expect users to read them.

@ifreund
ifreundforce-pushed the build-default-prefix branch from 1ff1bb2 to 06fa464CompareApril 29, 2021 21:15
@ifreund

Copy link
Copy Markdown
MemberAuthor

Implemented the changes to the proposal decided upon in the stage2 meeting just now.

@ifreundifreund changed the title std/build: change default install prefix from local zig-cache to .std/build: change default install prefix to zig-out Apr 29, 2021
Currently the default install prefix is $BUILD_ROOT/zig-cache,
but mixing cache and artifacts makes little sense. Instead make
$BUILD_ROOT/zig-out the default.
@ifreund
ifreundforce-pushed the build-default-prefix branch from 06fa464 to 5079d11CompareApril 29, 2021 21:59
@ifreund

Copy link
Copy Markdown
MemberAuthor

I decided to not yet implement a way to override the default in the interest of avoiding future breaking changes as the build system matures and new features become available. In particular, #7959 could significantly change the way paths are treated and allow for the defaults to be overridden in a more flexible way. Futhermore, we don't yet have a concrete motivating use-case for a way to override the default install prefix in the first place.

@andrewrk
andrewrk merged commit 4e07755 into ziglang:masterApr 30, 2021
@ifreund
ifreund deleted the build-default-prefix branch April 30, 2021 08:10
@daurnimator

Copy link
Copy Markdown
Contributor

Shouldn't this have been implemented more like a default DEST_DIR?

@ifreund

Copy link
Copy Markdown
MemberAuthor

@daurnimator What exactly do you think should be different?

tau-dev added a commit to tau-dev/zls that referenced this pull request May 13, 2021
alexnask added a commit to zigtools/zls that referenced this pull request May 13, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breakingImplementing this issue could cause existing code to no longer compile or have different behavior.zig build systemstd.Build, the build runner, `zig build` subcommand, package management

Projects

None yet

Development

Successfully merging this pull request may close these issues.

change the default installation directory from ./zig-cache to ./

7 participants

@ifreund@andrewrk@marler8997@Jarred-Sumner@jmc-88@g-w1@daurnimator