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

zig info command - print lib path, std path and global cache path #5394

Closed
wants to merge 18 commits into from

Conversation

Sergeeeek
Copy link
Contributor

@Sergeeeek Sergeeeek commented May 21, 2020

Implements the enhancement #4427

Creating this PR before everything is finished to get early feedback.

A couple of questions:

  1. stage0 uses Blake2 to compute the compiler id hash, can I use that for self hosted or does it need to be re-implemented? Will be implemented in Partially implement cache hash API in zig #4635
  2. How to pass a version string from build.zig? Are there any examples/docs? I haven't found anything. Will use addBuildOption

Progress

Self hosted:

  • Add an info command to zig
  • lib path
  • std path
  • global cache path (different for self hosted and stage1)
  • zig info --json
  • zig info --help

Example output:

$ zig info
lib_dir         	/home/sergeeeek/Work/zig/build/lib/zig
std_dir         	/home/sergeeeek/Work/zig/build/lib/zig/std
global_cache_dir	/home/sergeeeek/.cache/zig/(stage1|self_hosted)

$ zig info --json
{
  "lib_dir": "/home/sergeeeek/Work/zig/build/lib/zig",
  "std_dir": "/home/sergeeeek/Work/zig/build/lib/zig/std",
  "global_cache_dir": "/home/sergeeeek/.cache/zig/(stage1|self_hosted)"
}

$ zig info --help
Usage: zig info [options]

   Outputs path to zig lib dir, std dir and the global cache dir.

Options:
   --help                 Print this help and exit
   --json                 Output as json instead of a table format

Stage 1:

  • Add an info command to zig
  • Call the self-hosted info command, so the output is the same

@Sergeeeek Sergeeeek changed the title zig info - print lib path, std path and global cache path zig info command - print lib path, std path and global cache path May 21, 2020
@Sergeeeek
Copy link
Contributor Author

How to pass a version string from build.zig? Are there any examples/docs? I haven't found anything.

I'm thinking creating a separate file called version.txt at the root of the project and reading it from cmake and at comptime in self hosted.

I guess I'd also need to call git to get the revision inside a comptime, not sure if that's a good solution though.

Any help on choosing the best approach would be appreciated.

@Sergeeeek
Copy link
Contributor Author

Found the zig blake2 implementation, working on translating cache_hash.cpp into zig, which is used by current get_compiler_id

@andrewrk
Copy link
Member

Found the zig blake2 implementation, working on translating cache_hash.cpp into zig, which is used by current get_compiler_id

I'll work on merging #4635 soon, I think you might be duplicating efforts here

@ifreund
Copy link
Member

ifreund commented May 24, 2020

I'd argue that json output is overkill for this. A simple table would be easier to work with from shell scripts without depending on something like jq.

@data-man
Copy link
Contributor

A simple table would be easier to work

Or to add additional argument:
zig info [--json]

@Sergeeeek
Copy link
Contributor Author

Found the zig blake2 implementation, working on translating cache_hash.cpp into zig, which is used by current get_compiler_id

I'll work on merging #4635 soon, I think you might be duplicating efforts here

Oh didn't notice that PR, I'll stop then. Didn't get that far anyway :)

A simple table would be easier to work

Or to add additional argument:
zig info [--json]

Agreed, that's a good idea

@Sergeeeek
Copy link
Contributor Author

Sergeeeek commented May 25, 2020

Figuring out how to pass version from build.zig takes too long for me 😅

I'd like to split self hosted version resolution into a separate PR and merge this in its current state.

var json_format = false;
for (cmd_args) |arg| {
if (mem.eql(u8, arg, "--json")) {
json_format = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also have a --text option to set it back to non-json?

Perhaps it should be a --format=json argument instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's text by default, why add an additional --text argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's common in many command line tools so that aliases and defaults set in wrappers can be undone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a --format option, also changed error messages to better match other zig commands.

@Sergeeeek
Copy link
Contributor Author

Sergeeeek commented Jun 3, 2020

Found an inconsistency between Linux and OSX that affects this PR: fs.selfExePath() on Linux resolves symlinks and on OSX it doesn't.

The fix to that is very simple, I can just call os.readlinkZ on OSX like it currently does on linux. Can I include that in this PR?

@Sergeeeek
Copy link
Contributor Author

Actually, looks like symlinks in selfExePath are only resolved on Linux. Not sure what to do about it, but I would propose to resolve them on all platforms.

For example brew usually installs binaries into it's own location and then links them to /usr/local/bin, in which case zig info would stop working.

@Sergeeeek
Copy link
Contributor Author

Another solution is to remove link resolution in selfExePath on linux and do it inside zig info, but I'm not sure how much stuff is dependent on that behavior

Also some help text and error changes to better match the rest of zig
@andrewrk andrewrk closed this in e26dda5 Aug 18, 2020
@andrewrk
Copy link
Member

Thanks @Sergeeeek. I made some adjustments and then it landed in e26dda5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants