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

fix(devcontainer): remove -R in chown #13348

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Jul 16, 2024

Follow-up to #13111, might make the || true no longer necessary as well

Motivation

  • it seems like the -R isn't necessary??
    • that would take very long for me to run and without it I seemed to get a fresh build working fine

Modifications

  • remove the recursive -R from the chown command in pre-build.sh

Verification

Did a local rebuild
Will also confirm with a fresh Codespace of this branch (it's building right now)

  • EDIT: Made a fresh Codespace, which built and ran make start UI=true successfully (and the UI, Controller, Server et al were running correctly as well)

- it seems like the `-R` isn't necessary??
  - that would take very long for me to run and without it I seemed to get a fresh build working fine

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added the area/build Build or GithubAction/CI issues label Jul 16, 2024
@agilgur5 agilgur5 requested a review from Joibel July 16, 2024 00:41
@Joibel Joibel self-assigned this Jul 16, 2024
@agilgur5 agilgur5 added the area/contributing Contributing docs, ownership, etc. Also devtools like devcontainer and Nix label Jul 16, 2024
Copy link
Member

@Joibel Joibel left a comment

Choose a reason for hiding this comment

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

I can't work out if something has changed to mean this is fine now, but it's also fine for me.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 19, 2024
@Joibel Joibel merged commit 1f0adca into argoproj:main Jul 19, 2024
20 checks passed
@agilgur5 agilgur5 deleted the fix-devcontainer-chown branch July 19, 2024 11:50
Joibel added a commit to Joibel/argo-workflows that referenced this pull request Jul 22, 2024
argoproj#13348 removed a
recursive flag from chown. I believe this is because this is slow on
poorly performing filesystems.

This breaks codegen with
```
fatal: could not create leading directories of
'/home/vscode/go/src/github.com/gogo/protobuf': Permission denied
```

This is a smarter recursive chown which fixes the issue for me.

Signed-off-by: Alan Clucas <alan@clucas.org>
Joibel pushed a commit to pipekit/argo-workflows that referenced this pull request Sep 19, 2024
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Joibel pushed a commit that referenced this pull request Sep 20, 2024
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/contributing Contributing docs, ownership, etc. Also devtools like devcontainer and Nix lgtm This PR has been approved by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants