Skip to content

Fixed an error when using the command to download src #2940

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 10, 2024
Merged

Fixed an error when using the command to download src #2940

merged 3 commits into from
Jun 10, 2024

Conversation

waitstory
Copy link
Contributor

issue: #2938

Description

Fixed an error when using the command to download src

Which issue(s) this PR fixes:

Fixes #2938

Testing

Checklist:

  • I ran tests as well as code linting locally to verify my changes.
  • I have done manual verification of my changes, changes working as expected.

@waitstory
Fixed an error when using the command to download src
f3c033f 
issue: #2938

Signed-off-by: waitstory <waitstory@163.com>
@sweep-ai Sweep AI
Copy link

sweep-ai bot commented May 16, 2024 •
edited
Loading

Sweep: PR Review

Sweep has finished reviewing your pull request.

pkg/fission-cli/cmd/package/util/util.go

The changes import the os/exec package and replace the os.Rename function with the Unix mv command executed via exec.Command.

Potential Issues

  • Replacing os.Rename with exec.Command("mv", path, fileName) introduces a dependency on the Unix mv command, which may not be available or behave consistently across different environments.

    • fission/pkg/fission-cli/cmd/package/util/util.go

    Lines 221 to 222 in f3c033f

    cmd := exec.Command("mv", path, fileName)
    _, err = cmd.Output()

@sanketsudake
Copy link
Member

Just following golang/go#41487 so see if there is better way to handle this issue.

@waitstory
Copy link
Contributor Author

Just following golang/go#41487 so see if there is better way to handle this issue.就按照 golang/go#41487 看看是否有更好的方法来处理这个问题。

There seems to be no direct solution to the problem

sanketsudake
sanketsudake reviewed May 27, 2024
View reviewed changes
@sanketsudake
Copy link
Member

@waitstory Would you like to make the above change or should I raise a separate PR?

@waitstory
Copy link
Contributor Author

Let me make the change and submit it

@sanketsudake sanketsudake merged commit 9b978cf into fission:main Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanketsudake sanketsudake sanketsudake approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Download source code to file error when using fission command
2 participants
@waitstory @sanketsudake