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

Shell.mv doesn't move correctly #2293

Closed
fangyi-zhou opened this issue Apr 16, 2019 · 13 comments · Fixed by #2309
Closed

Shell.mv doesn't move correctly #2293

fangyi-zhou opened this issue Apr 16, 2019 · 13 comments · Fixed by #2309
Labels

Comments

@fangyi-zhou
Copy link
Contributor

Description

Shell.mv should behave like mv in shell, as described in documentation.

Repro steps

https://github.com/fangyi-zhou/fake-mv-bug

  • Create a file 1
  • Shell.mv "1" "2"
-> BuildFailedException: Target 'All' failed.
-> One or more errors occurred. (Could not find file '/Users/fangyi/fake-mv-bug/2'.)
-> FileNotFoundException: Could not find file '/Users/fangyi/fake-mv-bug/2'.

Expected behavior

Rename 1 to 2

Actual behavior

FileNotFoundException

Known workarounds

Shell.rename

Related information

FAKE 5.13

@matthid
Copy link
Member

matthid commented Apr 27, 2019

Wow, apparently either nobody is using this function or people just used the wrong order without caring too much...
Out of curiosity I dug very deep and found that this bug was introduced 5! years ago with a329bd3

Thanks for reporting :)
I guesss I'll fix this with a notice in the release notes, as it clearly is wrong (according to docs and parameter names) and it actually had this behavior previously...

@fangyi-zhou
Copy link
Contributor Author

I believe 5.13.3 does not fix the issue. Could you investigate?

@fangyi-zhou
Copy link
Contributor Author

fangyi-zhou commented Apr 30, 2019

fz315@cloud-vm-47-197 (master)✗ % ./fake.sh --version                                                                                                                                         ~/fake-mv-bug
FAKE 5 - F# Make (5.13.3) (this line is written to standard error, see https://github.com/fsharp/FAKE/issues/2066)
FakePath: /home/fz315/fake-mv-bug/.fake/.store/fake-cli/5.13.3/fake-cli/5.13.3/tools/netcoreapp2.1/any/Fake.Runtime.dll
Paket.Core: 5.203.2
fz315@cloud-vm-47-197 (master)✗ [130] % ./fake.sh build                                                                                                                                       ~/fake-mv-bug
The last restore is still up to date. Nothing left to do.
run All
Building project with version: LocalBuild
Shortened DependencyGraph for Target All:
<== All

The running order is:
Group - 1
  - All
Starting target 'All'
Finished (Failed) 'All' in 00:00:00.0108150

---------------------------------------------------------------------
Build Time Report
---------------------------------------------------------------------
Target     Duration
------     --------
All        00:00:00.0032824   (Could not find file '/home/fz315/fake-mv-bug/2'.)
Total:     00:00:00.2413778
Status:    Failure
---------------------------------------------------------------------
Script reported an error:
-> BuildFailedException: Target 'All' failed.
-> One or more errors occurred. (Could not find file '/home/fz315/fake-mv-bug/2'.)
-> FileNotFoundException: Could not find file '/home/fz315/fake-mv-bug/2'.
Hint: To further diagnose the problem you can run fake in verbose mode `fake -v run ...` or set the 'FAKE_DETAILED_ERRORS' environment variable to 'true'
Hint: Could not find a version in your paket.dependencies file, consider adding 'version 5.203.2' at the top of your dependencies file (/home/fz315/fake-mv-bug/paket.dependencies).
Read https://github.com/fsharp/FAKE/issues/2193 for details.
Performance:
 - Cli parsing: 1 second
 - Packages: 29 milliseconds
 - Script analyzing: 33 milliseconds
 - Script running: 656 milliseconds
 - Script cleanup: 1 millisecond
 - Runtime: 2 seconds

@matthid
Copy link
Member

matthid commented Apr 30, 2019

Maybe you need to update your packages... or delete build.fsx.lock

@fangyi-zhou
Copy link
Contributor Author

With updated lock file

fz315@cloud-vm-47-197 (master)✗ [127] % ./fake.sh build                                                                                                                                       ~/fake-mv-bug
The last restore is still up to date. Nothing left to do.
run All
Building project with version: LocalBuild
Shortened DependencyGraph for Target All:
<== All

The running order is:
Group - 1
  - All
Starting target 'All'
Finished (Failed) 'All' in 00:00:00.0121210

---------------------------------------------------------------------
Build Time Report
---------------------------------------------------------------------
Target     Duration
------     --------
All        00:00:00.0041858   (Could not find a part of the path '/home/fz315/fake-mv-bug/2/1'.)
Total:     00:00:00.3098645
Status:    Failure
---------------------------------------------------------------------
Script reported an error:
-> BuildFailedException: Target 'All' failed.
-> One or more errors occurred. (Could not find a part of the path '/home/fz315/fake-mv-bug/2/1'.)
-> DirectoryNotFoundException: Could not find a part of the path '/home/fz315/fake-mv-bug/2/1'.
Hint: To further diagnose the problem you can run fake in verbose mode `fake -v run ...` or set the 'FAKE_DETAILED_ERRORS' environment variable to 'true'
Hint: Could not find a version in your paket.dependencies file, consider adding 'version 5.203.2' at the top of your dependencies file (/home/fz315/fake-mv-bug/paket.dependencies).
Read https://github.com/fsharp/FAKE/issues/2193 for details.
Performance:
 - Cli parsing: 1 second
 - Packages: 27 milliseconds
 - Script analyzing: 34 milliseconds
 - Script running: 703 milliseconds
 - Script cleanup: 1 millisecond
 - Runtime: 2 seconds

@fangyi-zhou
Copy link
Contributor Author

it looks like it's treating 2 as a directory name instead of a file name.
Could you please re-open the issue?

@matthid matthid reopened this Apr 30, 2019
@matthid
Copy link
Member

matthid commented Apr 30, 2019

Should have written a test :(

@matthid
Copy link
Member

matthid commented Apr 30, 2019

In addition to this bug, you would expect Shell.mv to also work on directories, correct?

@fangyi-zhou
Copy link
Contributor Author

I believe so. According to documentation: "Like "mv" in a shell."

@matthid
Copy link
Member

matthid commented Apr 30, 2019

And also different behavior depending on whether "2" actually exists as a directory or not.

@fangyi-zhou
Copy link
Contributor Author

Yeah. According to the man page of mv, it does act both as renaming and moving.

@matthid matthid mentioned this issue Apr 30, 2019
@matthid
Copy link
Member

matthid commented Apr 30, 2019

I started this here: #2308
I'd assume some tests are missing as you can rename directories as well :/
So there is a lot of if/else logic missing ...

@fangyi-zhou fangyi-zhou mentioned this issue Apr 30, 2019
7 tasks
@matthid
Copy link
Member

matthid commented May 1, 2019

Should be released now :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants