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

vlang: Propagate build inputs, add $LDFLAGS #70466

Merged
merged 1 commit into from Oct 7, 2019
Merged

Conversation

Chiiruno
Copy link
Contributor

@Chiiruno Chiiruno commented Oct 5, 2019

Motivation for this change

Some examples weren't working.
Also for -compress option.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@veprbl
Copy link
Member

veprbl commented Oct 6, 2019

buildFHSUserEnv is for software that we can't patch. This is not the case for vlang.

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 6, 2019

I wasn't aware there was another way to fix this.
How do I do it?

@veprbl
Copy link
Member

veprbl commented Oct 6, 2019

@Chiiruno Usually for open source software we apply patches and/or simple wrappers to make things work the "Nix way".

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 6, 2019

@veprbl I did attempt to use a wrapper instead, unfortunately it simply would not work.
I looked through the source and I didn't find any hard references to /bin, so I'm not entirely sure what the issue is.
Perhaps vlang/v#1509 is related?

@veprbl
Copy link
Member

veprbl commented Oct 7, 2019

@Chiiruno You need to document the issue better. What doesn't work? How does it not work?
I can't reproduce the issue as described:

Some examples weren't working.

Everything I've tried compiles for me as long as I use
nix-shell -p vlang -p glfw -p freetype -p openssl

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 7, 2019

Because it doesn't work without that.
Sorry, it's been a busy day.

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 7, 2019

To elaborate, -compress does not work without upx and binutils.
On it's own, vlang will not compile programs that require glfw, freetype, or openssl without the fhs env.
I'll test your command real quick.

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 7, 2019

Okay, it works with that command.
Is there a way to get this to work without having to use that command however?

@veprbl
Copy link
Member

veprbl commented Oct 7, 2019

In principle there is nothing wrong with the current situation. It makes a perfect sense that if you want to compile something that uses, for example, the glfw, then you need to include the glfw dependency in your environment.

If V only supports limited number of libraries, it should be possible to make them all work out of the box. Perhaps putting them to the propagatedBuildInputs should be enough for most cases.

@Chiiruno
Copy link
Contributor Author

Chiiruno commented Oct 7, 2019

Okay, I had a fundamental misunderstanding when testing apparently.
Before, I tried propagatedBuildInputs and it didn't work, however I was calling /nix/store/qm8p5085lqq2flivwqmmmhjbd7sxlpn6-vlang-0.1.21/bin/v, and that was dumb, since it wasn't using the shell of the derivation, but my current system.
I used propagatedBuildInputs with nix-shell -p /nix/store/qm8p5085lqq2flivwqmmmhjbd7sxlpn6-vlang-0.1.21 and it works fine now.
Will update with this much simpler method.

@Chiiruno Chiiruno changed the title vlang: Fix dependency issues vlang: Propagate build inputs, add $LDFLAGS Oct 7, 2019
@veprbl
Copy link
Member

veprbl commented Oct 7, 2019

@GrahamcOfBorg build vlang

@veprbl veprbl merged commit 795b155 into NixOS:master Oct 7, 2019
@Chiiruno Chiiruno deleted the dev/vlang branch October 7, 2019 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants