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

use case: depending on system headers and libraries #2041

Open
Akuli opened this Issue Mar 10, 2019 · 21 comments

Comments

Projects
None yet
4 participants
@Akuli
Copy link
Contributor

Akuli commented Mar 10, 2019

On my system, @cImport(@cInclude("stdio.h")) works, but @cImport(@cInclude("gmp.h")) doesn't. gmp.h is in /usr/include/x86_64-linux-gnu, and stdio.h is in /usr/include. I'm running a simple test file with zig test filename.zig --library gmp --library c:

// run with:  zig test filename.zig --library gmp --library c
const gmp = @cImport(@cInclude("gmp.h"));

test "asdasd" {
    var num: gmp.mpz_t = undefined;
    gmp.mpz_init_set_ui(&num[0], 123);
    defer gmp.mpz_clear(&num[0]);
    _ = gmp.gmp_printf(c"'%Zd' should be '-123'\n", &num[0]);
}

This worked in 0.3.0+9c5852aa, but is broken in 0.3.0+0a8a7a57. On this system, zig is cloned to ~/sourcestuff/zig and built there, then bin and lib are copied to ~/zig/ (~/zig/bin is in $PATH).

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

If I do @cInclude("/usr/include/x86_64-linux-gnu/gmp.h"), I get a different error: lld: error: unable to find library -lgmp. gmp is installed with apt.

akuli@Akuli-Desktop:~$ apt show libgmp3-dev 
Package: libgmp3-dev
Version: 2:6.1.2+dfsg-1
Priority: optional
Section: libdevel
Source: gmp
Maintainer: Debian Science Team <debian-science-maintainers@lists.alioth.debian.org>
Installed-Size: 21,5 kB
Depends: libgmp-dev (= 2:6.1.2+dfsg-1)
Conflicts: libgmp10-dev (<< 2:5.0.1+dfsg-7)
Replaces: libgmp10-dev
Homepage: http://gmplib.org/
Tag: devel::lang:c, devel::library, field::mathematics, implemented-in::c,
 role::devel-lib, suite::gnu
Download-Size: 15,1 kB
APT-Sources: http://pkgmaster.devuan.org/merged ascii/main amd64 Packages
Description: Multiprecision arithmetic library developers tools

akuli@Akuli-Desktop:~$ 

Akuli added a commit to Akuli/asda that referenced this issue Mar 10, 2019

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

This works:

zig test filename.zig --library gmp --library c --library-path /usr/lib/x86_64-linux-gnu/ -isystem /usr/include/x86_64-linux-gnu/

I still liked the old behaviour where I didn't need to specify the paths verbosely

@andrewrk andrewrk added this to the 0.5.0 milestone Mar 10, 2019

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 10, 2019

This is an important topic moving forward. I recognize that for some use cases, this is a step backwards in convenience. Whereas before, @cImport would automatically look in system-dependent locations, now, it will not access any system directories unless explicitly requested. Zig is moving towards system-independent builds, which is great for cross compiling, and for the upcoming package manager, but it's less convenient for depending on system-installed packages, as this issue notes.

However, depending on system headers and libraries is an important use case, as long as it's intentional.

Note: stdio.h works for you because your target is x86_64-linux-gnu, which is one of the targets that now has libc out-of-the-box. See #514

I see a few different directions this could take, and I'm not sure which one is best yet.

  • Zig CLI could include system headers and libraries by default, and we could have a -nostdinc parameter, mirroring clang's, that disables this functionality.
  • Keep Zig CLI status quo, and in order to depend on system headers and libraries, you would have to use a command line parameter --enable-system-search-paths (or something like that)
  • Zig CLI would not have the ability to search system paths. That would fall onto the zig build system, and you would do something like const gmp = b.findSystemLibrary("gmp", "gmp.h"); exe.linkLibrary(gmp);.
  • Maybe we could do the first bullet point, but zig build system always passes -nostdinc and so you'd have to do the previous bullet point within the zig build system.

@andrewrk andrewrk changed the title cInclude doesn't find header file, worked in an older version of zig use case: depending on system headers and libraries Mar 10, 2019

@andrewrk andrewrk modified the milestones: 0.5.0, 0.4.0 Mar 10, 2019

@thejoshwolfe

This comment has been minimized.

Copy link
Member

thejoshwolfe commented Mar 10, 2019

Assuming the zig package manager works well, then i think the third bullet is best; make it explicit when you're depending on specific system dependencies. And projects that want to be friendly to the zig package manager would be encouraged to never do that.

EDIT: i take it back. i don't think i understand the usecase for this. i need to do more research.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

Boooooo. How in the world is this better than the old behaviour? I don't see how having to hard-code the path of a header file that includes the architechture name is "system-independent builds".

I still have no idea how to figure out the correct command-line arguments from a build script anyway. If that's not possible from within a build.zig, I'll just create a build.sh or something.

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 10, 2019

@Akuli did you see the first bullet point? I'm pretty sure that's exactly what you want, no?

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

Yes, but I don't understand how the change gives more "system-independent builds".

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 10, 2019

The thing that got harder to do (and is an open issue because I recognize that it is a problem) is depend on your system's gmp library. So the fact that you're getting an error for trying to do a system-dependent build makes it harder to do on accident. You're getting an error for the thing that is preventing your code from working on more systems.

This will make a lot more sense once we have a working package manager, but given that we don't have that yet, I think it probably does make sense to do the first bullet point for the 0.4.0 release. It doesn't make sense to deprecate the "old way" of doing things until the new way is available.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

Yes, the first bullet point would be nice.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 10, 2019

Can someone ELI5 how relying on the system's gmp.h is "system-dependent"? If I write something like "install GMP with your favorite package manager" to my README, I expect everyone to install it before attempting to compile my code. As far as I can tell, using the system's package manager is a very cross-platform way to install C libraries.

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 10, 2019

Here are some things to consider:

  • The version installed on one system may not be compatible with the version installed on another system.
  • The package may be missing from the package manager on some systems.
  • The package may have different names on different systems. Example: https://github.com/ziglang/zig/blob/0.3.0/cmake/Findllvm.cmake#L11
  • On Windows, there isn't really a system package manager, and it's likely to require hard coded search paths equivalent to --library-path /usr/lib/x86_64-linux-gnu/ -isystem /usr/include/x86_64-linux-gnu/. Even the first bullet point of #2041 (comment) wouldn't solve this; it sort of just gives posix systems special treatment that isn't available for Windows.
  • System packages may be patched in unpredictable or problematic ways. For one example see #2001. Another example: NixOS/nixpkgs#55397

These are all considerations that apply to depending on system headers and libraries, and, importantly, they do not apply to zig package manager packages, or to source files that you bundle with your own source.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 11, 2019

Thanks for patiently explaining all the things to me. Now I see why it's better to specify --library-path and -isystem explicitly for applications that are meant to work on Windows. The old behaviour would be nice for non-Windows projects though, and any of the bullet points would be fine for that.

With the current zig, how do I write a build.zig that can take those arguments? I don't want to put any system-dependent information in my build.zig, and I don't know how else I could do it, so I'm thinking of creating a shell script that generates build.zig and runs that. Is there a nicer way?

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 11, 2019

Another thought: Will projects like GMP be in zig's package manager whenever that will be ready? Otherwise it's useless for this, and instead of solving the problems with system-wide installed dependencies I would need system-wide dependencies and some extra gibberish to worry about.

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 11, 2019

With the current zig, how do I write a build.zig that can take those arguments?

With current Zig, I hope you can make do temporarily with this workaround where you specify some hard coded search paths. You can use addLibPath and addIncludeDir in build.zig. Here are 2 examples:

zig/build.zig

Line 161 in 83b7952

lib_exe_obj.addIncludeDir(include_path);

zig/build.zig

Line 135 in 83b7952

lib_exe_obj.addLibPath(lib_dir);

I don't want to put any system-dependent information in my build.zig, and I don't know how else I could do it, so I'm thinking of creating a shell script that generates build.zig and runs that. Is there a nicer way?

I don't think that should be necessary - worst case scenario you could have build.zig do whatever your shell script was going to do. I recognize that the hard coded path workaround is an issue, but do you think it could be an acceptable workaround for a week or two until I resolve this issue?

Another thought: Will projects like GMP be in zig's package manager whenever that will be ready?

Yes absolutely. GMP is a great candidate to be a zig package.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 11, 2019

worst case scenario you could have build.zig do whatever your shell script was going to do

I was talking about something like this:

// this is build-template.zig
...
lib_exe_obj.addIncludeDir("INCLUDEDIR");
lib_exe_obj.addLibPath("LIBPATH");
...
#!/bin/bash
# this is build.sh
includedir="$1"
libpath="$2"
cp build-template.zig build.zig
sed -i "s:INCLUDEDIR:$includedir:" build.zig
sed -i "s:LIBPATH:$libpath:" build.zig
zig build

Now that I think about this a bit more, I could use environment variables instead of this. Maybe I'll do that.

@thejoshwolfe

This comment has been minimized.

Copy link
Member

thejoshwolfe commented Mar 11, 2019

it looks like your build.sh accepts parameters. why not have build.zig accept the parameters instead?

In general, anything you can do in a bash script you can also do in pure zig[1], so there should never[1] be a usecase for generating zig code.

[1] To be pedantic, there are cases for generating zig code, but they are very specific such as generating struct definitions. This is an unsolved issue in zig's design. See the "refiy" discussions for more info on that one specifically.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 11, 2019

why not have build.zig accept the parameters instead?

I tried to ask about this earlier:

With the current zig, how do I write a build.zig that can take those arguments?

Nobody told me how to do it.

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 11, 2019

Here's an example of adding build options:

zig/build.zig

Line 68 in a909efa

const skip_release = b.option(bool, "skip-release", "Main test suite skips release builds") orelse false;

You can see how to specify them with zig build --help. You can choose []const u8 instead of bool for paths.

@Akuli

This comment has been minimized.

Copy link
Contributor Author

Akuli commented Mar 11, 2019

This looks nice. Thanks!

Akuli added a commit to Akuli/asda that referenced this issue Mar 11, 2019

andrewrk added a commit that referenced this issue Mar 11, 2019

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 11, 2019

In the above commit, I made it so that if you call linkSystemLibrary in a build script, and the system library isn't libc, and the target is native, then it adds native target search paths automatically. Idea being that if you call linkSystemLibrary, clearly the intent is to include native system search paths when trying to link.

I'm going to leave this issue open for reconsideration in the 0.5.0 milestone. Once we have the zig package manager I think it will be worth revisiting what is the workflow for both the use cases of trying to depend on system things, and trying to avoid system dependencies.

It also may make sense to move this logic to the Zig CLI, or maybe make the way that libc is specified special. I want to leave this open and think about it for a while. But for now, if you use zig build then linkSystemLibrary will make your code able to find system headers and libraries.

Note to self: remember to double check or add a test for a transitive dependency on system paths. For example if one artifact links a system library, and then another one links the first artifact.

@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Mar 11, 2019

@tiehuis

This comment has been minimized.

Copy link
Member

tiehuis commented Apr 3, 2019

I think it would be valuable to expose the ability to search system include paths from the CLI. I'm happy with the non-default searching as outlined in your previous comment regarding the --enable-system-search-paths option.

I've had to write some trivial build scripts already for a few cases which is mildly annoying. I personally can make do, but I imagine that this would be a moderately common barrier for newer users or those which want to share a quick example which happens to utilize system c libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.