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

docs(contrib): note windows symbolic link config #12604

Closed
wants to merge 2 commits into from

Conversation

tczhao
Copy link
Member

@tczhao tczhao commented Feb 1, 2024

Fixes #12603

In Windows, setting symlink=true in Git automatically trigger powershell command mklink on all symbolic link files, so they connect properly in windows file system

Setting autocrlf to false prevents line return getting modified causing bash issue

Motivation

Modifications

Verification

Copy link

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

I believe these can be set automatically in the .gitatrributes if necessary, which would be preferable to docs changes.

autocrlf is usually recommended as auto though

docs/running-locally.md Outdated Show resolved Hide resolved
@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs area/build Build or GithubAction/CI issues labels Feb 1, 2024
@agilgur5 agilgur5 self-assigned this Feb 1, 2024
Co-authored-by: Anton Gilgur <[email protected]>
Signed-off-by: Tianchu Zhao <[email protected]>
@tczhao
Copy link
Member Author

tczhao commented Feb 1, 2024

I believe these can be set automatically in the .gitatrributes if necessary, which would be preferable to docs changes.

autocrlf is usually recommended as auto though

  • For the symlink, if we configure it using gitattributes, we have to specify every single symbolic files but with git config core.symlinks, it will automatically set up symlink for all files, and we don't need to maintain this file list.

  • For autocrlf, I think setting to auto if only useful if we develop frontend without running the backend. Running the argo-workflow backend in Windows requires dev container or wsl2; both require line return using LF. Otherwise, bash commands won't work.
    Since in the running-locally guide we always run the backend and frontend together, I think setting autocrlf=false for this repo is a valid approach.

@agilgur5
Copy link

agilgur5 commented Feb 9, 2024

  • For the symlink, if we configure it using gitattributes, we have to specify every single symbolic files but with git config core.symlinks, it will automatically set up symlink for all files, and we don't need to maintain this file list.

Looks like core.symlinks is true by default?

  • [...] Since in the running-locally guide we always run the backend and frontend together, I think setting autocrlf=false for this repo is a valid approach.

So it would make sense to set this in the .gitattributes then, no?

Also, I do sometimes run the front-end build or install separately (not the devserver, that I do run together), but I'm either native on Mac or in the devcontainer.

@agilgur5
Copy link

agilgur5 commented Mar 1, 2024

Looks like core.symlinks is true by default?

Stumbled upon on an old thread I hadn't finished reading -- another contributor there also had to enable core.symlinks. I'm wondering if it's maybe disabled on Windows installations by default? (though that might depend on where you installed from too)

@tczhao
Copy link
Member Author

tczhao commented Mar 1, 2024

Looks like core.symlinks is true by default?

Stumbled upon on an old thread I hadn't finished reading -- another contributor there also had to enable core.symlinks. I'm wondering if it's maybe disabled on Windows installations by default? (though that might depend on where you installed from too)

I had a quick look, and the latest Git installation in Windows defaults the symlink to enabled. I think we only see this issue if people have been using Git on Windows and never reinstalled it.
Perhaps we could make a comment and workaround label in the issue and close it instead

@tczhao tczhao closed this Mar 14, 2024
@agilgur5 agilgur5 added the area/contributing Contributing docs, ownership, etc. Also devtools like devcontainer and Nix label Apr 26, 2024
@agilgur5 agilgur5 added the area/windows Windows Container support label Jun 6, 2024
@agilgur5
Copy link

Superseded by #13534

@agilgur5 agilgur5 added the solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) label Aug 31, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators Aug 31, 2024
@agilgur5 agilgur5 changed the title docs: document windows symbolic link config for local dev docs(contrib): note windows symbolic link config for local dev Aug 31, 2024
@agilgur5 agilgur5 changed the title docs(contrib): note windows symbolic link config for local dev docs(contrib): note windows symbolic link config Aug 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/build Build or GithubAction/CI issues area/contributing Contributing docs, ownership, etc. Also devtools like devcontainer and Nix area/docs Incorrect, missing, or mistakes in docs area/windows Windows Container support solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make argoexec-image returns error in Windows expected 'package', found signal_darwin
2 participants