GlobalProtect-OpenConnect: init at 1.2.6 #106465

jerith666 · 2020-12-09T16:24:24Z

Things done

jerith666 · 2020-12-09T16:25:23Z

cc openconnect maintainers @pradeepchhetri @tricktron

nixos/modules/services/networking/globalprotect-vpn.nix

pkgs/tools/networking/globalprotect-openconnect/default.nix

pkgs/tools/networking/openconnect/default.nix

jerith666 · 2020-12-10T02:32:55Z

Thanks for the thorough review, @SuperSandro2000. I did some fetching and rebasing so I could be clear on which of your suggestions were semantic versus only formatting. I've included the semantic suggestions and the two fixes you suggested (OS = "Darwin" and passing the darwin PCSC framework individually) in this PR.

With regard to the formatting changes, I've put them on a separate branch: jerith666/nixpkgs@globalprotect-vpn...jerith666:globalprotect-vpn-formatting. Is there a style guide you're following that led you to these changes, which I could keep in mind for future work? Or are they more of your personal preference?

SuperSandro2000 · 2020-12-11T05:38:47Z

Is there a style guide you're following that led you to these changes, which I could keep in mind for future work? Or are they more of your personal preference?

Not really. Normally things are defined in the order they are used at build time.

Also please fix the eval issue.

jerith666 · 2020-12-12T16:16:15Z

Whoops, missed the eval issue! (In fact there was a second one lurking behind the first ...) 🤦 Fixed now.

I've left the formatting changes on their own branch. IMO the code reads more clearly without them, but it's obviously a matter of opinion and I'm totally fine if you'd like to merge them (or probably better, squash-merge them). :)

SuperSandro2000

Can you please squash the globalprotect-openconnect related commits into one init commit?

SuperSandro2000 · 2020-12-20T15:38:52Z

pkgs/tools/networking/openconnect/default.nix

+
+      wrapProgram $out/bin/vpnc-script \
+        --set OS ${os} \
+        --prefix PATH : "${stdenv.lib.makeBinPath [ nettools gawk openresolv coreutils gnugrep ]}";


Suggested change

--prefix PATH : "${stdenv.lib.makeBinPath [ nettools gawk openresolv coreutils gnugrep ]}";

--prefix PATH : "${stdenv.lib.makeBinPath [ nettools gawk openresolv coreutils gnugrep]}";

this and all other formatting suggestions are in the latest force-push.

pkgs/tools/networking/globalprotect-openconnect/default.nix

SuperSandro2000 · 2020-12-20T15:43:51Z

@jerith666 I am not sure why this causes so many rebuilds but please target staging.

SuperSandro2000 · 2020-12-20T15:43:55Z

@ofborg eval

jerith666 · 2020-12-21T21:41:47Z

I was also surprised that this ended up rebuilding a bunch of stuff for me locally. One that I remember being surprised about is gnucash. So I used nix-store -q --graph to figure it out:

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep 'openconnect.*->'
"/nix/store/2p3w750xzn398kgl3hy0almaxf6i1w5f-openconnect-8.10.drv" -> "/nix/store/4hp0c8rhn57w7ylr8i61nasgfky3h75b-fix-paths.patch.drv" [color = "magenta"];
"/nix/store/bpgwzg2lb1id41v3kjlj0rid4zw86plb-openconnect-8.10.tar.gz.drv" -> "/nix/store/2p3w750xzn398kgl3hy0almaxf6i1w5f-openconnect-8.10.drv" [color = "green"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep '4hp0c.*->'
"/nix/store/4hp0c8rhn57w7ylr8i61nasgfky3h75b-fix-paths.patch.drv" -> "/nix/store/prv3bchmk48rvhfflznd496x9k2lbjkm-network-manager-1.26.0.drv" [color = "burlywood"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep 'prv3b.*->'
"/nix/store/prv3bchmk48rvhfflznd496x9k2lbjkm-network-manager-1.26.0.drv" -> "/nix/store/swgcbiq4rd9n69k6v4f17w0yv73mnkpp-libproxy-0.4.15.drv" [color = "blue"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep 'swgcb.*->'
"/nix/store/swgcbiq4rd9n69k6v4f17w0yv73mnkpp-libproxy-0.4.15.drv" -> "/nix/store/adc29252yixs60q7c5mjx5qv5sihy290-glib-networking-2.66.0.drv" [color = "red"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep 'adc292.*->'
"/nix/store/adc29252yixs60q7c5mjx5qv5sihy290-glib-networking-2.66.0.drv" -> "/nix/store/12q49n28niihvdcsc37z11llwp97cds7-geoclue-2.5.6.drv" [color = "burlywood"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep '12q49n.*->'
"/nix/store/12q49n28niihvdcsc37z11llwp97cds7-geoclue-2.5.6.drv" -> "/nix/store/7rnb1lz0aybng93lb6mpvsw7vbfhf2df-webkitgtk-2.30.1.drv" [color = "red"];

$ nix-store -q --graph $(nix-store -qd $(type -p gnucash)) | grep '7rnb1l.*->'
"/nix/store/7rnb1lz0aybng93lb6mpvsw7vbfhf2df-webkitgtk-2.30.1.drv" -> "/nix/store/3b1p0n2vi651q1nf510va24j5cqgla3q-gnucash-4.2.drv" [color = "red"];

Nothing in that chain seems obviously wrong to me, so I'll re-target to staging.

SuperSandro2000

please fix the merge conflict

pkgs/tools/networking/openconnect/default.nix

jerith666 · 2021-04-16T15:00:33Z

Just pinging this, I think all the requested changes have been made and it's okay to merge?

tricktron

@jerith666 Works on my Mac with Big Sur 11.4 and looks good to me.

@SuperSandro2000 Ready to merge?

.version

tricktron

@jerith666 approved.
@SuperSandro2000 Next try: Ready to merge😃?

veprbl · 2021-06-02T05:06:18Z

Merge remote-tracking branch 'origin/nixos-unstable' into globalprotect-vpn

We don't really allow merging PRs that contain merge commits.

veprbl · 2021-06-02T05:06:34Z

Please rebase.

tricktron · 2021-06-02T06:56:54Z

@veprbl Good spotting, thanks.

@jerith666 Ready for another (hopefully the last) round😄? Could you remove the merge commit and rebase it onto master?

nixos/modules/services/networking/globalprotect-vpn.nix

@SuperSandro2000

suggested by @SuperSandro2000

Co-authored-by: Sandro <[email protected]>

jerith666 · 2021-06-02T22:53:31Z

Rebased without the merge commit. The only outstanding issue is the LANG in the systemd service definition, I believe.

sternenseemann · 2021-06-02T23:04:27Z

Rebased without the merge commit. The only outstanding issue is the LANG in the systemd service definition, I believe.

Have you tested the service without it?

Co-authored-by: Sandro <[email protected]> Co-authored-by: sterni <[email protected]>

@liff

adds resolvectl support; as suggested by @liff

jerith666 · 2021-06-02T23:24:01Z

Tested without the LANG setting, it works; new version force-pushed just now. Maybe it's finally ready? 🤞 😄

tricktron

Still working on my Mac.

@SuperSandro2000 Next try: Ready to merge?

tricktron · 2021-06-05T16:08:34Z

@jerith666 Congrats, you did it😃!

jerith666 · 2021-06-05T17:10:30Z

Awesome, thanks everyone! Hopefully my machine won't be too bored now that all these rebuilds will be cached ... ;-)

lopsided98 · 2021-06-28T01:23:52Z

The NixOS module is missing mkIf cfg.enable, so it is always enabled.

jerith666 · 2021-06-28T01:59:12Z

Gack -- whoops, good catch! Fix is in #128473.

ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package labels Dec 9, 2020

ofborg bot requested a review from tricktron December 9, 2020 16:39

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 501-1000 labels Dec 9, 2020

jerith666 changed the title ~~Globalprotect vpn~~ GlobalProtect-OpenConnect: init at 1.2.6 Dec 9, 2020

jerith666 force-pushed the globalprotect-vpn branch from a5da358 to aa1a94a Compare December 9, 2020 16:58

SuperSandro2000 reviewed Dec 9, 2020

View reviewed changes

jerith666 force-pushed the globalprotect-vpn branch from d6dffe9 to 2cc336b Compare December 10, 2020 02:25

jerith666 force-pushed the globalprotect-vpn branch 2 times, most recently from 01d565d to a2171d5 Compare December 12, 2020 16:15

SuperSandro2000 reviewed Dec 20, 2020

View reviewed changes

jerith666 changed the base branch from master to staging December 21, 2020 21:42

jerith666 force-pushed the globalprotect-vpn branch from a2171d5 to c78f897 Compare December 21, 2020 21:49

SuperSandro2000 requested changes Jan 18, 2021

View reviewed changes

pkgs/tools/networking/openconnect/default.nix Outdated Show resolved Hide resolved

SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021

jerith666 force-pushed the globalprotect-vpn branch 2 times, most recently from 417f68c to 362a22d Compare January 19, 2021 01:46

jerith666 removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 19, 2021

SuperSandro2000 requested a review from Mic92 January 19, 2021 12:47

tricktron approved these changes Jun 1, 2021

View reviewed changes

SuperSandro2000 reviewed Jun 1, 2021

View reviewed changes

.version Outdated Show resolved Hide resolved

jerith666 force-pushed the globalprotect-vpn branch from 28d520e to 440eccb Compare June 1, 2021 19:28

ofborg bot requested a review from tricktron June 1, 2021 20:19

tricktron approved these changes Jun 2, 2021

View reviewed changes

sternenseemann reviewed Jun 2, 2021

View reviewed changes

adisbladis removed their request for review June 2, 2021 16:22

jerith666 and others added 3 commits June 2, 2021 18:37

openconnect: fix homepage

ab12136

openconnect: pass darwin framework individually

c2e076a

suggested by @SuperSandro2000

openconnect: adapt vpnc-script for use in nix

6160d73

Co-authored-by: Sandro <[email protected]>

jerith666 force-pushed the globalprotect-vpn branch from c5b0c88 to 44de2f3 Compare June 2, 2021 22:52

ofborg bot requested a review from tricktron June 2, 2021 22:56

jerith666 and others added 2 commits June 2, 2021 19:22

globalprotect-openconnect: init at 1.2.6

e2b7cfe

Co-authored-by: Sandro <[email protected]> Co-authored-by: sterni <[email protected]>

vpnc-scripts: 2020-02-21 -> 2021-03-21

12368c0

adds resolvectl support; as suggested by @liff

jerith666 force-pushed the globalprotect-vpn branch from 44de2f3 to 12368c0 Compare June 2, 2021 23:22

tricktron approved these changes Jun 4, 2021

View reviewed changes

SuperSandro2000 merged commit ef45f53 into NixOS:staging Jun 5, 2021

jerith666 mentioned this pull request Jun 11, 2021

README.md: add section for NixOS yuezk/GlobalProtect-openconnect#65

Merged

jerith666 mentioned this pull request Jun 28, 2021

globalprotect-vpn: add missing 'mkIf cfg.enable' #128473

Merged

11 tasks

kurnevsky mentioned this pull request Oct 25, 2021

openconnect: hardcoded paths in vpnc script makes resolvconf is always using the fallback #105020

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GlobalProtect-OpenConnect: init at 1.2.6 #106465

GlobalProtect-OpenConnect: init at 1.2.6 #106465

jerith666 commented Dec 9, 2020

jerith666 commented Dec 9, 2020

jerith666 commented Dec 10, 2020

SuperSandro2000 commented Dec 11, 2020 •

edited

Loading

jerith666 commented Dec 12, 2020

SuperSandro2000 left a comment

SuperSandro2000 Dec 20, 2020

jerith666 Dec 21, 2020

SuperSandro2000 commented Dec 20, 2020

SuperSandro2000 commented Dec 20, 2020

jerith666 commented Dec 21, 2020

SuperSandro2000 left a comment

jerith666 commented Apr 16, 2021

tricktron left a comment •

edited

Loading

tricktron left a comment

veprbl commented Jun 2, 2021

veprbl commented Jun 2, 2021

tricktron commented Jun 2, 2021

jerith666 commented Jun 2, 2021

sternenseemann commented Jun 2, 2021

jerith666 commented Jun 2, 2021

tricktron left a comment

tricktron commented Jun 5, 2021

jerith666 commented Jun 5, 2021

lopsided98 commented Jun 28, 2021 •

edited

Loading

jerith666 commented Jun 28, 2021

	--prefix PATH : "${stdenv.lib.makeBinPath [ nettools gawk openresolv coreutils gnugrep ]}";
	--prefix PATH : "${stdenv.lib.makeBinPath [ nettools gawk openresolv coreutils gnugrep]}";

GlobalProtect-OpenConnect: init at 1.2.6 #106465

GlobalProtect-OpenConnect: init at 1.2.6 #106465

Conversation

jerith666 commented Dec 9, 2020

Things done

jerith666 commented Dec 9, 2020

jerith666 commented Dec 10, 2020

SuperSandro2000 commented Dec 11, 2020 • edited Loading

jerith666 commented Dec 12, 2020

SuperSandro2000 left a comment

Choose a reason for hiding this comment

SuperSandro2000 Dec 20, 2020

Choose a reason for hiding this comment

jerith666 Dec 21, 2020

Choose a reason for hiding this comment

SuperSandro2000 commented Dec 20, 2020

SuperSandro2000 commented Dec 20, 2020

jerith666 commented Dec 21, 2020

SuperSandro2000 left a comment

Choose a reason for hiding this comment

jerith666 commented Apr 16, 2021

tricktron left a comment • edited Loading

Choose a reason for hiding this comment

tricktron left a comment

Choose a reason for hiding this comment

veprbl commented Jun 2, 2021

veprbl commented Jun 2, 2021

tricktron commented Jun 2, 2021

jerith666 commented Jun 2, 2021

sternenseemann commented Jun 2, 2021

jerith666 commented Jun 2, 2021

tricktron left a comment

Choose a reason for hiding this comment

tricktron commented Jun 5, 2021

jerith666 commented Jun 5, 2021

lopsided98 commented Jun 28, 2021 • edited Loading

jerith666 commented Jun 28, 2021

SuperSandro2000 commented Dec 11, 2020 •

edited

Loading

tricktron left a comment •

edited

Loading

lopsided98 commented Jun 28, 2021 •

edited

Loading