build(client): use Taskfile for tun2socks #1967

fortuna · 2024-04-11T20:38:00Z

This is nice:

Also, we can have different behavior based on the host platform very easily. And test preconditions as well. See my code.

I believe we can replace our run_action with Taskfile instead.

Details: https://taskfile.dev/

You can run the tasks with go run github.com/go-task/task/v3/cmd/task, or we can build it with go build github.com/go-task/task/v3/cmd/task, then just call task.

This PR doesn't move any files, including sources and build output.

daniellacosse · 2024-04-12T17:11:46Z

Taskfile.yml

+run: when_changed
+
+includes:
+  client:tun2socks: ./client/src/tun2socks/Taskfile.yml


note to self that we'll need to change this to "backend" if that other PR is merged

daniellacosse

a few thoughts

daniellacosse · 2024-04-12T17:16:14Z

client/src/tun2socks/Taskfile.yml

+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      - |
+        {{if ne OS "windows"}}GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC='zig cc -target x86_64-windows' {{end}}go build {{.ELECTRON_BUILD_FLAGS}} -o '{{.TARGET_DIR}}/tun2socks.exe' '{{.ELECTRON_MAIN_PKG}}'


Is there a way we can space this out to make it more readable? ie

{{if ne OS "windows"}} GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC='zig cc -target x86_64-windows' \ {{end}} go build {{.ELECTRON_BUILD_FLAGS}} -o '{{.TARGET_DIR}}/tun2socks.exe' '{{.ELECTRON_MAIN_PKG}}'

or maybe it makes sense to load env or taskfiles based on the environment in a "setup environment" task

I did the reformatting you suggested.

daniellacosse · 2024-04-12T17:19:42Z

client/src/tun2socks/Taskfile.yml

+        msg: "Must set ANDROID_NDK"
+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # -androidapi should match the minSdkVersion that the Android client supports.


should we hoist the androidapi into a var?

I like that, it's cleaner. Done.

daniellacosse · 2024-04-12T17:20:16Z

client/src/tun2socks/Taskfile.yml

+      TARGET_DIR: '{{.OUT_DIR}}/ios'
+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # -iosversion should match the target version that the iOS client supports.


same with iosversion

daniellacosse · 2024-04-12T17:20:32Z

client/src/tun2socks/Taskfile.yml

+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # MACOSX_DEPLOYMENT_TARGET and -iosversion should match the versions that the macOS client supports.
+      - export MACOSX_DEPLOYMENT_TARGET=10.14; {{.GOMOBILE_BIND_CMD}} -target=macos,maccatalyst -iosversion=13.1 -bundleid org.outline.tun2socks -o '{{.TARGET_DIR}}/Tun2socks.xcframework' '{{.TASKFILE_DIR}}/outline/tun2socks' '{{.TASKFILE_DIR}}/outline/shadowsocks'


and macosx deployment target

daniellacosse · 2024-04-12T17:21:33Z

client/src/tun2socks/Taskfile.yml

+
+  clean:
+    cmds:
+      - rm -r "{{.REPO_ROOT}}/output/client/tun2socks" .task


can we add this clean to the root package.json clean?

Good idea. Done.

fortuna

Comments addressed

fortuna · 2024-04-12T17:38:01Z

client/src/tun2socks/Taskfile.yml

+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      - |
+        {{if ne OS "windows"}}GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC='zig cc -target x86_64-windows' {{end}}go build {{.ELECTRON_BUILD_FLAGS}} -o '{{.TARGET_DIR}}/tun2socks.exe' '{{.ELECTRON_MAIN_PKG}}'


I did the reformatting you suggested.

fortuna · 2024-04-12T17:38:18Z

client/src/tun2socks/Taskfile.yml

+        msg: "Must set ANDROID_NDK"
+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # -androidapi should match the minSdkVersion that the Android client supports.


I like that, it's cleaner. Done.

fortuna · 2024-04-12T17:40:57Z

client/src/tun2socks/Taskfile.yml

+      TARGET_DIR: '{{.OUT_DIR}}/ios'
+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # -iosversion should match the target version that the iOS client supports.


fortuna · 2024-04-12T17:41:00Z

client/src/tun2socks/Taskfile.yml

+    cmds:
+      - rm -rf "{{.TARGET_DIR}}" && mkdir -p "{{.TARGET_DIR}}"
+      # MACOSX_DEPLOYMENT_TARGET and -iosversion should match the versions that the macOS client supports.
+      - export MACOSX_DEPLOYMENT_TARGET=10.14; {{.GOMOBILE_BIND_CMD}} -target=macos,maccatalyst -iosversion=13.1 -bundleid org.outline.tun2socks -o '{{.TARGET_DIR}}/Tun2socks.xcframework' '{{.TASKFILE_DIR}}/outline/tun2socks' '{{.TASKFILE_DIR}}/outline/shadowsocks'


fortuna · 2024-04-12T17:41:08Z

client/src/tun2socks/Taskfile.yml

+
+  clean:
+    cmds:
+      - rm -r "{{.REPO_ROOT}}/output/client/tun2socks" .task


Good idea. Done.

daniellacosse · 2024-04-12T18:21:47Z

Just make sure to fix the breaking build

fortuna · 2024-04-12T18:43:19Z

client/src/tun2socks/Taskfile.yml

+      # -w Omit the DWARF symbol table.
+      # -X Set the value of the string variable.
+      - |
+        {{if ne OS .TARGET_OS -}}


Ugh, it turns out OS is a function, not a variable, so it doesn't take the .. That was breaking the build.

…file (#1997) This PR fixes several path issues in electron caused by the recent source code movement and output path change in #1967 .

Use Taskfile

abcc450

github-actions bot added the size/M label Apr 11, 2024

Update description

d173d1a

fortuna changed the title ~~build(client): use Taskfile for tun2socks.~~ build(client): use Taskfile for tun2socks Apr 11, 2024

Improve

b808880

github-actions bot added size/L and removed size/M labels Apr 11, 2024

fortuna added 3 commits April 11, 2024 18:07

Remove os

0d64343

Fix

ede26cd

Better

3688bb0

fortuna requested review from daniellacosse and jyyi1 April 11, 2024 23:06

fortuna marked this pull request as ready for review April 11, 2024 23:06

fortuna requested a review from a team as a code owner April 11, 2024 23:06

Merge branch 'master' into fortuna-taskfile2

c31df07

fortuna mentioned this pull request Apr 11, 2024

DO NOT SUBMIT build(client): use Taskfile for tun2socks #1966

Closed

fortuna added 2 commits April 12, 2024 12:09

Simplify build.action.mjs

3c4a5d2

Tweaks

f6216c2

daniellacosse reviewed Apr 12, 2024

View reviewed changes

fortuna added 2 commits April 12, 2024 13:22

Refactor Electron

cd49beb

Address review

fb392eb

fortuna commented Apr 12, 2024

View reviewed changes

fortuna requested a review from daniellacosse April 12, 2024 17:44

fortuna added 2 commits April 12, 2024 13:55

Remove .task

0626979

Fix clean

d98760d

daniellacosse approved these changes Apr 12, 2024

View reviewed changes

fortuna added 2 commits April 12, 2024 14:22

Fix os check

43fdc9a

Fix

90133f8

Fix

c9eb496

fortuna commented Apr 12, 2024

View reviewed changes

fortuna merged commit 685ef88 into master Apr 12, 2024
19 checks passed

fortuna deleted the fortuna-taskfile2 branch April 12, 2024 19:15

jyyi1 mentioned this pull request Apr 24, 2024

fix(client/electron): update electron's tun2socks paths built by taskfile #1997

Merged

jyyi1 added a commit that referenced this pull request Apr 24, 2024

fix(client/electron): update electron's tun2socks paths built by task…

64f8ed1

…file (#1997) This PR fixes several path issues in electron caused by the recent source code movement and output path change in #1967 .

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(client): use Taskfile for tun2socks #1967

build(client): use Taskfile for tun2socks #1967

fortuna commented Apr 11, 2024 •

edited

Loading

daniellacosse Apr 12, 2024 •

edited

Loading

daniellacosse left a comment

daniellacosse Apr 12, 2024

daniellacosse Apr 12, 2024

fortuna Apr 12, 2024

daniellacosse Apr 12, 2024

fortuna Apr 12, 2024

daniellacosse Apr 12, 2024

fortuna Apr 12, 2024

daniellacosse Apr 12, 2024

fortuna Apr 12, 2024

daniellacosse Apr 12, 2024

fortuna Apr 12, 2024

fortuna left a comment

fortuna Apr 12, 2024

fortuna Apr 12, 2024

fortuna Apr 12, 2024

fortuna Apr 12, 2024

fortuna Apr 12, 2024

daniellacosse commented Apr 12, 2024

fortuna Apr 12, 2024

build(client): use Taskfile for tun2socks #1967

build(client): use Taskfile for tun2socks #1967

Conversation

fortuna commented Apr 11, 2024 • edited Loading

daniellacosse Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

daniellacosse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortuna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniellacosse commented Apr 12, 2024

Choose a reason for hiding this comment

fortuna commented Apr 11, 2024 •

edited

Loading

daniellacosse Apr 12, 2024 •

edited

Loading