Skip to content

command/run: fix concurrent writes to the flags list - #335

Merged
igungor merged 4 commits into
masterfrom
safecmd
Aug 5, 2021
Merged

igungor merged 4 commits into
masterfrom
safecmd

Conversation

@igungor

@igungor igungor commented Aug 3, 2021

Copy link
Copy Markdown
Member

The run command iterates over the commands list, where each item is a
pointer to the command. urfave/cli packages implicitly adds -h flag
to each command, which introduces data race because s5cmd executes
them in parallel.

Fixes #301

The `run` command iterates over the commands list, where each item is a
pointer to the command. `urfave/cli` packages implicitly adds `-h` flag
to each command, which introduces data race because `s5cmd` executes
them in parallel.

Fixes #301
@igungor
igungor requested a review from sonmezonur August 3, 2021 13:50
@igungor

igungor commented Aug 3, 2021

Copy link
Copy Markdown
Member Author

@ocakhasan

@igungor
igungor marked this pull request as ready for review August 3, 2021 15:52
@igungor
igungor requested review from a team and aykutfarsak and removed request for a team August 3, 2021 15:52
@igungor

igungor commented Aug 5, 2021

Copy link
Copy Markdown
Member Author

@aykutfarsak ping

@igungor
igungor merged commit babc8d5 into master Aug 5, 2021
@igungor
igungor deleted the safecmd branch August 5, 2021 15:40
@nelhage

nelhage commented Aug 5, 2021

Copy link
Copy Markdown
Contributor

Would you mind cutting a release containing this fix? I'd love to pull it in and re-try the code paths we'd disabled due to #301, and it would be easier for me to do so with a release.

@igungor

igungor commented Aug 13, 2021

Copy link
Copy Markdown
Member Author

Would you mind cutting a release containing this fix? I'd love to pull it in and re-try the code paths we'd disabled due to #301, and it would be easier for me to do so with a release.

We've released v1.4.0-rc.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: panic: cp flag redefined: help from s5cmd run

4 participants