build: use Task #1535

fortuna · 2024-04-16T07:10:40Z

We now build outline-ss-server directly and cross-compile the docker image.

sbruens

Can you get rid of scripts/run_action.sh now?

sbruens · 2024-04-16T13:46:20Z

README.md

@@ -41,7 +41,7 @@ See [Shadowsocks resistance against detection and blocking](docs/shadowsocks.md)
 1. **Start the server**

   ```sh
-   npm run action shadowbox/server/start
+   task shadowbox:server:start


Add Task as a prerequisite above for easy setup instructions.

We need to mention Go as well.

Added Go. The task binary is now built on npm install

sbruens · 2024-04-16T13:49:30Z

src/shadowbox/README.md

     ```

   - **Docker Container**

     ```sh
-     npm run action shadowbox/docker/start
+     task shadowbox:docker:start


This task doesn't exist. You only specified docker:build. Can you create one?

Ah, I forgot about it. I'll work on it.

sbruens · 2024-04-16T13:56:27Z

src/shadowbox/Taskfile.yml

+      # Copy entrypoint command
+      - cp '{{joinPath .TASKFILE_DIR "docker/cmd.sh"}}' '{{.IMAGE_ROOT}}/'
+      # Build image with given root
+      - podman build --force-rm


Honestly, this will be surprising to most users - Docker is still exponentially more popular. A user trying to run the server will run task shadowbox:docker:build thinking they're building an image using their Docker installation, and it fails on a missing podman installation, which isn't mentioned anywhere. If nothing else, update the README to replace Docker with Podman.

However, as many users will have Docker installed already, an additional installation could just put them off trying out the instructions to play with the server. Maybe we can explicitly create a shadowbox:podman:build or check if podman exists and fallback to Docker if not.

PS: I'm not arguing that Docker is better than Podman (it's not), I'm just hoping to keep the on-ramp for new users easy and predictable.

I agree. It was for testing, I didn't actually intend to change. I've reverted to docker, but have an option to override. I can now do ./task shadowbox:docker:build DOCKER=podman

Nice, the override is cool.

jyyi1

This is cool. Maybe we can support building from Windows once we migrate to a cross-platform build pipeline in the future.

jyyi1 · 2024-04-16T20:14:08Z

src/metrics_server/README.md

  ```
 - Integration test
  ```sh
-  npm run action metrics_server/test_integration
+  task metrics_server:integration:test


nit shall we rename it to integration_test? Cuz we don't have other sub-tasks for integration?

Makes sense. That aligns with shadowbox too. Renamed

jyyi1 · 2024-04-16T20:15:15Z

src/metrics_server/Taskfile.yml

+    deps: [build]
+    cmds:
+      - cp '{{.TASKFILE_DIR}}/config_dev.json' '{{.SERVER_DIR}}/config.json'
+      - npx node '{{.SERVER_DIR}}/index.js'


Do we need npx here? Can we just use node {{.SERVER_DIR}}/index.js?

jyyi1 · 2024-04-16T20:17:34Z

README.md

@@ -41,7 +41,7 @@ See [Shadowsocks resistance against detection and blocking](docs/shadowsocks.md)
 1. **Start the server**

   ```sh
-   npm run action shadowbox/server/start
+   task shadowbox:server:start


We need to mention Go as well.

jyyi1 · 2024-04-16T20:29:14Z

src/shadowbox/Taskfile.yml

+      TARGET_OS: '{{.TARGET_OS | default "linux"}}'
+      TARGET_ARCH: '{{.TARGET_ARCH | default "x86_64"}}'
+      GOOS: '{{get (dict "macos" "darwin") .TARGET_OS | default .TARGET_OS}}'
+      GOARCH: '{{get (dict "x86_64" "arm64") .TARGET_ARCH | default .TARGET_ARCH}}'


Suggested change

GOARCH: '{{get (dict "x86_64" "arm64") .TARGET_ARCH | default .TARGET_ARCH}}'

GOARCH: '{{get (dict "x86_64" "amd64") .TARGET_ARCH | default .TARGET_ARCH}}'

sbruens

LGTM but please don't forget to add the docker:start task.

jyyi1

The code LGTM, please make sure the CI job passes.

jyyi1 · 2024-04-17T21:23:21Z

Taskfile.yml

+  clean:
+    desc: Clean output files
+    cmds:
+      - rm -rf .task src/*/node_modules/ build/ node_modules/ third_party/shellcheck/download/ third_party/*/bin third_party/jsign/*.jar


Do we need to remove the ./task binary built by npm install as well?

Makes sense. Done

fortuna · 2024-04-17T22:20:08Z

docker:start added. It was a pain to make it work. Ran into cross-compilation issues...

fortuna added 2 commits April 16, 2024 03:07

Use Task

84bbaf0

Fix hook

0b807bf

fortuna requested a review from a team as a code owner April 16, 2024 07:10

github-actions bot added the size/XXL label Apr 16, 2024

Fix lint

49bb526

sbruens requested changes Apr 16, 2024

View reviewed changes

jyyi1 reviewed Apr 16, 2024

View reviewed changes

fortuna added 7 commits April 16, 2024 17:01

Cleanup

cb009e8

Fix worflow

1734a81

Some fixes

21ff270

Fixes

fdf4d2c

FIX

b914ad2

Remove run_action

3763264

Fix test

8de4b7a

fortuna requested review from jyyi1 and sbruens April 17, 2024 14:38

Fix

9924c0c

sbruens approved these changes Apr 17, 2024

View reviewed changes

fortuna added 6 commits April 17, 2024 12:35

Fixes

9bcfd72

Add docker:start

ca85c39

Avoid temp

bddd958

Use 127.0.0.1

ec2599a

Debug

8971b16

Change order

6c69dec

jyyi1 approved these changes Apr 17, 2024

View reviewed changes

fortuna added 5 commits April 17, 2024 17:39

Swap order

4a596e8

Revert

6caf7c0

Statically link

7af91b5

Delete task

bdcd12d

Rename folders

72bb4a7

fortuna force-pushed the fortuna-task branch from ce1f362 to 72bb4a7 Compare April 17, 2024 22:14

fortuna merged commit 6556d7e into master Apr 17, 2024
10 checks passed

fortuna deleted the fortuna-task branch April 17, 2024 23:48

sbruens mentioned this pull request Aug 30, 2024

fix(server): fix version parsing #1586

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: use Task #1535

build: use Task #1535

fortuna commented Apr 16, 2024

sbruens left a comment

sbruens Apr 16, 2024

jyyi1 Apr 16, 2024

fortuna Apr 16, 2024

sbruens Apr 16, 2024

fortuna Apr 16, 2024

sbruens Apr 16, 2024

fortuna Apr 16, 2024

sbruens Apr 17, 2024

jyyi1 left a comment

jyyi1 Apr 16, 2024

fortuna Apr 16, 2024

jyyi1 Apr 16, 2024

fortuna Apr 16, 2024

jyyi1 Apr 16, 2024

jyyi1 Apr 16, 2024

fortuna Apr 16, 2024

sbruens left a comment

jyyi1 left a comment

jyyi1 Apr 17, 2024

fortuna Apr 17, 2024

fortuna commented Apr 17, 2024

	GOARCH: '{{get (dict "x86_64" "arm64") .TARGET_ARCH \| default .TARGET_ARCH}}'
	GOARCH: '{{get (dict "x86_64" "amd64") .TARGET_ARCH \| default .TARGET_ARCH}}'

build: use Task #1535

build: use Task #1535

Conversation

fortuna commented Apr 16, 2024

sbruens 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

jyyi1 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

sbruens left a comment

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortuna commented Apr 17, 2024