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

make file/dirnm comparison in relativeFilename case-insensitive for WIN32 #1893

Merged
merged 1 commit into from
Oct 8, 2018
Merged

make file/dirnm comparison in relativeFilename case-insensitive for WIN32 #1893

merged 1 commit into from
Oct 8, 2018

Conversation

fwmechanic
Copy link
Contributor

Issue

Running recent https://github.com/universal-ctags/ctags-win32/releases builds including ctags-2018-09-25_619a6fac-x64

  • on Windows 10 (x64), I observe that ctags writes tag (output) file having input fields containing absolute file paths, and
  • on Windows 8.1 (x64), running the same binary, tag file input fields contain relative paths (which is IMO correct/documented behavior).

This behavior occurs with --tag-relative=yes and --tag-relative=always.

Diagnosis

(using MinGW builds of current master branch) showed that getcwd function called by setCurrentDirectory is returning a lowercase drive-letter on Win10, and a uppercase drive-letter on Win8.1 (I can think of no explanation for this behavior, as I assume this routine is statically linked into ctags.exe; while diagnosing this issue I copied locally-built MinGW ctags.exe binaries from Win10 to Win8.1 host and observed same results as when using the ctags-win32/releases builds per above).

The existing ctags function relativeFilename (case-sensitively) compares this (setCurrentDirectory) value against the output of absoluteFilename( tagfile-name ) in order to determine the relative path; this comparison fails if there is a mismatch on the 0th character (in WIN32: the drive-letter); absoluteFilename contains WIN32-specific code that forces the drive-letter of its return value to uppercase, causing its output to match the (uppercase) drive-letter of Win8.1 setCurrentDirectory value (allowing a common path-prefix to be detected, and a valid relative path constructed), but to always mismatch the (lowercase) drive-letter of Win10 setCurrentDirectory value (preventing construction of a relative path, the issue).

Offered Fix

Because I wish to avoid falling into the tarpit of adding more/competing code to ensure (or assume) a specific case for the WIN32 drive-letter character (when the underlying filesystem is case-insensitive!?), the fix in this PR takes the approach of changing the filename comparison operation performed in relativeFilename to be case-insensitive on WIN32, a behavior which I believe to be in alignment with the characteristics of filesystem on that platform.

@coveralls
Copy link

coveralls commented Sep 28, 2018

Coverage Status

Coverage remained the same at 84.882% when pulling 004231d on fwmechanic:win32-case-insensitive-fnm-cmp into 208e2c0 on universal-ctags:master.

@masatake
Copy link
Member

masatake commented Oct 1, 2018

Thank you for making a pull request.
Could you put the first comment of this pull request to your commit log?
So people can know the background of this change with git log.

Issue

Running recent https://github.com/universal-ctags/ctags-win32/releases
builds including ctags-2018-09-25_619a6fac-x64

* on Windows 10 (x64), I observe that ctags writes tag (output) file
  having input fields containing absolute file paths, and
* on Windows 8.1 (x64), running the same binary, tag file input fields
  contain relative paths (which is IMO correct/documented behavior).

This behavior occurs with --tag-relative=yes and --tag-relative=always.

Diagnosis

(using MinGW builds of current master branch) showed that getcwd function
called by setCurrentDirectory is returning a lowercase drive-letter on
Win10, and a uppercase drive-letter on Win8.1 (I can think of no
explanation for this behavior, as I assume this routine is statically
linked into ctags.exe; while diagnosing this issue I copied locally-built
MinGW ctags.exe binaries from Win10 to Win8.1 host and observed same
results as when using the ctags-win32/releases builds per above).

The existing ctags function relativeFilename (case-sensitively) compares
this (setCurrentDirectory) value against the output of
absoluteFilename( tagfile-name ) in order to determine the relative path;
this comparison fails if there is a mismatch on the 0th character (in
WIN32 this is the drive-letter); absoluteFilename contains WIN32-specific
code that forces the drive-letter of its return value to uppercase,
causing its output to match the (uppercase) drive-letter of Win8.1
setCurrentDirectory value (allowing a common path-prefix to be detected,
and a valid relative path constructed), but to always mismatch the
(lowercase) drive-letter of Win10 setCurrentDirectory value (preventing
construction of a relative path, the issue).

Offered Fix

Because I wish to avoid falling into the tarpit of adding more/competing
code to ensure (or assume) a specific case for the WIN32 drive-letter
character (when the (WIN32 API to the) underlying filesystem is case-
insensitive!), the fix in this PR takes the approach of changing the
filename comparison operation performed in relativeFilename to be case-
insensitive on WIN32, a behavior which I believe to be in alignment with
the characteristics of filesystem on that platform.
@fwmechanic
Copy link
Contributor Author

I just force-pushed a comment-revision-only rewrite of the PR commit per request.

@masatake masatake merged commit 2888a6d into universal-ctags:master Oct 8, 2018
@masatake
Copy link
Member

masatake commented Oct 8, 2018

Thank you !

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.

None yet

3 participants