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

Add sstp client. #6064

Merged
merged 1 commit into from
Jan 31, 2015
Merged

Add sstp client. #6064

merged 1 commit into from
Jan 31, 2015

Conversation

ktosiek
Copy link
Contributor

@ktosiek ktosiek commented Jan 29, 2015

This adds sstp-client - VPN client for Microsoft's SSTP protocol. I've tested this with my job VPN, and it works without problems.

One thing I'm not sure about in the expression is extending configureFlags in preConfigure, but just adding a flag with "$out" to the configureFlags attribute didn't work (it seems like literal "$out" was passed in the ./configure arguments) - is there some better way to do this?

@lucabrunox
Copy link
Contributor

No in fact you should be adding --with-pppd-plugin-dir=$out/lib/pppd to the configureFlags. Try $(out).

@peti
Copy link
Member

peti commented Jan 30, 2015

IMHO the current solution in preConfigure is just fine. Since the script is guaranteed to be interpreted by bash, one could even use the shorter

configureFlags+=" --with-pppd-plugin-dir=$out/lib/pppd"

version, but that's a matter of taste.

@lucabrunox
Copy link
Contributor

@peti he used both, so just use one of the two :)

@peti
Copy link
Member

peti commented Jan 30, 2015

@lethalman, I'm not sure what you mean? As far as I can tell, the expression sets the --with-pppd-plugin-dir flag in one place only?

@lucabrunox
Copy link
Contributor

@peti I mean he's using also configureFlags, he could just move everything in preConfigure instead.

@peti
Copy link
Member

peti commented Jan 30, 2015

Ah, you are right. The current expression works fine, but still it's nice to have all configure definitions in one place.

@ktosiek
Copy link
Contributor Author

ktosiek commented Jan 30, 2015

I've amended the commit to use $(out) in configureFlags (BTW - why does this work when $out doesn't?)

@lucabrunox
Copy link
Contributor

output path ‘/nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz’ should have sha256 hash ‘0kpwywbavmlgid07rk8ff0bxp75bnfa1nc28w4j0pkxjhmja5n6k’, instead has ‘0b0l0lbmv7svj36b7j0l3yi74s8qg85bf04a95jivji3f51z96jc’

@ktosiek
Copy link
Contributor Author

ktosiek commented Jan 30, 2015

@lethalman that's strange - I've just removed that path and forced a rebuild, and it still worked. Is it possible that sourceforge serves us different files?

@7c6f434c
Copy link
Member

I've amended the commit to use $(out) in configureFlags (BTW - why does this work when $out doesn't?)

Because $out is effectively escaped (prevented from being interpreted as
a shell expansion) by Nix and stdenv code, and so is $(out). But the
latter is immediately interpreted by make, so it happens to work.

@lucabrunox
Copy link
Contributor

@ktosiek uhm possible

@ktosiek
Copy link
Contributor Author

ktosiek commented Jan 31, 2015

This is on commit 99c0af0 (same as proposed here), git status shows no local changes.
I've just run:

nix-store --delete /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz
P=$(nix-build . -A sstp); nix-store -q --roots "$P" | xargs rm; nix-store --delete "$P"
nix-build . -A sstp

It downloaded sstp from http://prdownloads.sourceforge.net/sstp-client/sstp-client/1.0.9/sstp-client-1.0.9.tar.gz (creating the /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz path again), and built sstp.

I've also tried:
ls -l /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz; sha256sum /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz

-r--r--r-- 1 root nixbld 3243308 Jan  1  1970 /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz
d3d8a26485b2cf0b24e148301b94b3ab9cdb17700ecd7c408b8fd6ad16f7fc4e  /nix/store/4shvfgcafmjs3l2gvzy2n68rn82lga51-sstp-client-1.0.9.tar.gz

@lethalman can you compare this with your downloaded file? I have no idea why the hash might be different for you :-<

lucabrunox pushed a commit that referenced this pull request Jan 31, 2015
@lucabrunox lucabrunox merged commit 87377ce into NixOS:master Jan 31, 2015
@lucabrunox
Copy link
Contributor

Ok works now, don't know what happened before, but I think it's some sourceforge mirror problem. Built locally, thanks.

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.

5 participants