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

refactor(client): move client/go output into root #2109

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

daniellacosse
Copy link
Contributor

@daniellacosse daniellacosse commented Aug 7, 2024

hopefully this won't conflict with your work @fortuna

this basically moves the Tun2socks build result out of cilent/output and into the root output folder

unpackedAppPath(), 'client', 'output', 'build',
(isWindows ? 'windows' : 'linux'),
'tun2socks' + (isWindows ? '.exe' : ''));
unpackedAppPath(),
Copy link
Contributor Author

@daniellacosse daniellacosse Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyyi1 do you know if this will work? do you mind testing it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should all be able to validate the builds. This is an example where the tun-only mode would help. It's one of our biggest pain points. We need to prioritize that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

@daniellacosse daniellacosse added the needs test Pull requests that require tests label Aug 7, 2024
@daniellacosse daniellacosse marked this pull request as ready for review August 7, 2024 16:04
@daniellacosse daniellacosse requested review from a team as code owners August 7, 2024 16:04
@daniellacosse daniellacosse marked this pull request as draft August 7, 2024 18:03
@daniellacosse daniellacosse marked this pull request as ready for review August 8, 2024 18:14
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we need to validate

unpackedAppPath(), 'client', 'output', 'build',
(isWindows ? 'windows' : 'linux'),
'tun2socks' + (isWindows ? '.exe' : ''));
unpackedAppPath(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should all be able to validate the builds. This is an example where the tun-only mode would help. It's one of our biggest pain points. We need to prioritize that.

client/go/Taskfile.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, provided it works.

Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux is not working (tun2socks is not included in the app, and output folder is not in app.asar.unpacked either, you can easily check the content in this folder after build: > outline-apps/output/client/electron/build/linux-unpacked/resources/app.asar.unpacked):

code: 'ENOENT',
syscall: 'spawn /tmp/.mount_OutlinWCuipE/resources/app.asar.unpacked/output/build/client/linux/tun2socks',

Neither do Windows:

code: 'ENOENT',
syscall: 'spawn C:\\Program Files (x86)\\outline-client\\resources\\app.asar.unpacked\\output\\build\\client\\windows\\tun2socks.exe',

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs test Pull requests that require tests size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants