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

Windows: brename doesn't detect when files will be overwritten if the replacement only differs in case #28

Closed
tspivey opened this issue Mar 21, 2023 · 9 comments
Labels

Comments

@tspivey
Copy link

tspivey commented Mar 21, 2023

Using brename v2.12.0 on Windows.
Create two files, test.txt and test.doc.

brename -x -p "\..+$" -r "" -d

[INFO] checking: [ ok ] 'test.doc' -> 'test'
[ERRO] checking: [ overwriting newly renamed path ] 'test.txt' -> 'test'
[ERRO] 1 potential error(s) detected, please check

This works as expected. Now create test.txt and Test.doc.

brename -x -p "\..+$" -r "" -d

[INFO] checking: [ ok ] 'Test.doc' -> 'Test'
[INFO] checking: [ ok ] 'test.txt' -> 'test'
[INFO] 2 path(s) to be renamed

Now do the rename:

brename -x -p "\..+$" -r ""

[INFO] renamed: 'Test.doc' -> 'Test'
[INFO] renamed: 'test.txt' -> 'test'
[INFO] 2 path(s) renamed

Only one file remains in the directory, because NTFS is case insensitive.

@shenwei356 shenwei356 added the bug label Mar 22, 2023
@shenwei356
Copy link
Owner

Thanks for reporting, and I've added a flag -w/--case-insensitive-path for correctly checking file overwrites in file systems like NTFS (mostly in Windows).

Right now, we show the warning message if it's not set in all operating systems. Should I make it on default for the Windows version?


$ brename -x -p "\..+$" -r "" -i -f ^Test -d
[INFO] main options:
[INFO]   ignore case: true
[INFO]   search pattern: (?i)\..+$
[INFO]      skip filters: ^\.
[INFO]   include filters: ^Test
[WARN] 
[WARN] If the file system where the search path locates is NTFS (mostly in Windows),
[WARN] please use -w/--case-insensitive-path to correctly check file overwrites!
[WARN] 
[INFO]   search paths: ./
[INFO] 
[INFO] checking: [ ok ] 'Test.doc' -> 'Test'
[INFO] checking: [ ok ] 'test.txt' -> 'test'
[INFO] 2 path(s) to be renamed

shenwei356 added a commit that referenced this issue Mar 22, 2023
@shenwei356
Copy link
Owner

shenwei356 commented Mar 22, 2023

Hmm, I think it should be set by default on Windows.

image

On Linux, we also show the warning message in case users are operating on an external disk with NTFS.

brename -x -p "\..+$" -r "" -i -f ^Test -d 
[WARN] 
[WARN] If the file system where the search path locates is NTFS (mostly on Windows),
[WARN] please use -w/--case-insensitive-path to correctly check file overwrites!
[WARN] 
[INFO] main options:
[INFO]   path filters and search pattern:
[INFO]         ignore case: true
[INFO]      search pattern: (?i)\..+$
[INFO]         replacement: 
[INFO] 
[INFO]        skip filters: ^\.
[INFO]     include filters: ^Test
[INFO] 
[INFO]   path overwrite checking:
[INFO]     case-insensitive path: false
[INFO]            overwrite mode: 0 (reporting error)
[INFO] 
[INFO] ------------------------------------------------------
[INFO] 
[INFO] search paths: ./
[INFO] 
[INFO] checking: [ ok ] 'Test.doc' -> 'Test'
[INFO] checking: [ ok ] 'test.txt' -> 'test'
[INFO] 2 path(s) to be renamed

@tspivey
Copy link
Author

tspivey commented Mar 24, 2023

Thanks for fixing this. Some edge cases:

  1. It's possible to make a folder case sensitive on Windows (fsutil file setCaseSensitiveInfo pathname). I don't know where this is used.
  2. Renaming a file strictly by case should probably be considered not an error (e.g. test.TXT to test.txt).

@shenwei356
Copy link
Owner

shenwei356 commented Mar 27, 2023

For case 1, I also noticed the setting somewhere too. As you said, it's an edge case. For most cases, we would better assume Windows users use case-intensive file systems. Then, for these professional users who can run tools like fsutil, we can offer another flag to disable the flag of -w/--case-insensitive-path, like -W/--case-sensitive-path.


For case 2, renaming test.TXT to test.txt should be stopped in case-sensitive filesystems when text.txt exists.

[ERRO] checking: [ new path existed ] 'test.TXT' -> 'test.txt'

Updates: You're right. In my test, it reported the new path existed error even when only one file exists. I'm testing this in a virtual Windows machine where the disk is partitioned as EXT4! The file path package in Golang thinks the file exists already.

@shenwei356
Copy link
Owner

shenwei356 commented Mar 27, 2023

I've added the -W/--case-sensitive-path and the case 2 is fixed.

Warnings:
  1. The path in file systems like NTFS is case-insensitive, so you should switch on the flag
     -w/--case-insensitive-path to correctly check file overwrites.
  2. The flag -w/--case-insensitive-path is switched on by default on Windows, please use
     -W/--case-sensitive-path to disable it if the file system is indeed case sensitive.

Flags:
  -w, --case-insensitive-path     the file system (e.g., NTFS) is case-insensitive. It's automatically
                                  swiched on on Windows
  -W, --case-sensitive-path       believing that the file system is case-sensitive. Please use this to
                                  disable the flag -w/--case-insensitive-path, which is switched on by
                                  default on Windows

Here are some tests on Windows with two files (test.TXT and Test.doc) in one directory. The file system is ext2/ext3 according to the information from stat -f; actually, it's ext4.

test 1

image

Since it's ext4, I believe the paths should be case-sensitive, so I add -W.

image

test 2

image

Please help to test it. (commit 8b708c6)

@shenwei356
Copy link
Owner

@tspivey I'll tag a new release if it works as expected :)

@tspivey
Copy link
Author

tspivey commented Apr 3, 2023

I verified:

  1. with test.txt and Test.doc, removing the extension fails because the files would conflict in case.
  2. I could rename test.TXT to test.txt without problems.

I'm trying to come up with some edge cases reading the official documentation.

Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not. However, it is acceptable to specify a period as the first character of a name. For example, ".temp".

This might be a separate issue since it doesn't have anything to do with case, but it can also cause data loss on NTFS. if I create test.txt and test..txt, then do:
brename -p ".txt$" -r ""
I'm left with test, and one of the files is lost.

@shenwei356
Copy link
Owner

with test.txt and Test.doc, removing the extension fails because the files would conflict in case.

OK, it's exptected.

I could rename test.TXT to test.txt without problems.

Cool.

Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not.

🤮 Have to add another rule ...

@shenwei356
Copy link
Owner

Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not.

OK, they are not allowed now.

$ brename -p '.txt' -d
[ERRO] checking: [ new path ending with a period ] 'test..txt' -> 'test.'
[INFO] checking: [ ok ] 'test.txt' -> 'test'
[ERRO] 1 potential error(s) detected, please check

A warning message is added in the help message.

Warnings:
  1. The path in file systems like FAT32 or NTFS is case-insensitive, so you should switch on the flag
     -w/--case-insensitive-path to correctly check file overwrites.
  2. The flag -w/--case-insensitive-path is switched on by default on Windows, please use
     -W/--case-sensitive-path to disable it if the file system is indeed case-sensitive.
  3. New paths ending with a period of space, being error-prone, are not allowed.

brename_windows_amd64.exe.tar.gz

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

No branches or pull requests

2 participants