Warn on case mismatch on Unix systems #24860

EmrysMyrddin · 2019-01-09T09:16:40Z

Purpose

This pull request aims to fix #23441 (display a warning when using a file with the wrong case).

Exemples

# Foo.gd

extends Node

# res://Bar.gd

extends 'foo.gd'

This shows this warning :

Case mismatch opening requested file '/Users/valentin/Dev/Projects/Godot/Tests/foo.gd', stored as '/Users/valentin/Dev/Projects/Godot/Tests/Foo.gd' in the filesystem. This file will not open when exported to other case-sensitive platforms.

Caveats

Since I'm using POSIX realpath function, this warning will also be displayed if there is a symlink somewhere in the path. This is due to real_path resolving all symlinks to find a real path on the disk.

Exemple:

# linked/Foo.gd

extends Node

# Bar.gd

extends linked/Foo.gd

Here, given that linked folder is a symlink, we will get this warning :

Case mismatch opening requested file '/Users/valentin/Dev/Projects/Godot/Tests/linked/Foo.gd', stored as '/Users/valentin/Dev/Projects/Godot/Tests/somewhere_else/Foo.gd' in the filesystem. This file will not open when exported to other case-sensitive platforms.

I haven't found any way to avoid this problem... So if anyone have an idea...

EmrysMyrddin · 2019-01-09T09:26:59Z

There was already on open PR (#23441) fo this issue, but since I haven't reused any code from it, I have opened a new one to avoid messing with git history.

EmrysMyrddin · 2019-01-09T09:32:52Z

Also notice that this is my first contribution and that I'm not a C++ expert.

For example, realpath is doing a malloc and return a pointer which should be freed after use. From what I have understood of the implementation of String, the pointer should be automatically cleaned up but I'm not sure...

Kanabenki · 2019-01-10T08:17:17Z

If I remember correctly, I couldn't use realpath for #23503 because while it has the correct behaviour on macOS, it is not guaranteed on Linux that the resolved path will have the correct case for the file, I will test it again later to confirm.

Anyway thanks for taking a look at this !

EmrysMyrddin · 2019-01-10T12:47:29Z

Yes, you are right. I have tried it on my Ubuntu...

I'm running out of clean solution...

Perhaps this can be a first solution for at least macOS ? Since linux is unlikely to be case insensitive.

akien-mga · 2019-05-30T14:27:10Z

I couldn't use realpath for #23503 because while it has the correct behaviour on macOS, it is not guaranteed on Linux that the resolved path will have the correct case for the file

Why doesn't the resolved path have the correct case on Linux? I don't see much in man realpath that would indicate that.

akien-mga · 2019-05-30T14:31:26Z

Oops, I tried it on my Linux system and indeed it seems to report unwanted things:

WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/hid-0018:04F3:25BF.0005-battery/type', stored as '/sys/devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-8/i2c-ELAN2514:00/0018:04F3:25BF.0005/power_supply/hid-0018:04F3:25BF.0005-battery/type' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/hid-0018:04F3:25BF.0005-battery/present', stored as '/sys/devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-8/i2c-ELAN2514:00/0018:04F3:25BF.0005/power_supply/hid-0018:04F3:25BF.0005-battery/present' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/hid-0018:04F3:25BF.0005-battery/status', stored as '/sys/devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-8/i2c-ELAN2514:00/0018:04F3:25BF.0005/power_supply/hid-0018:04F3:25BF.0005-battery/status' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/hid-0018:04F3:25BF.0005-battery/capacity', stored as '/sys/devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-8/i2c-ELAN2514:00/0018:04F3:25BF.0005/power_supply/hid-0018:04F3:25BF.0005-battery/capacity' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/ADP1/type', stored as '/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0003:00/power_supply/ADP1/type' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/BAT0/type', stored as '/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0/type' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/BAT0/present', stored as '/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0/present' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/BAT0/status', stored as '/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0/status' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.
WARNING: _open: Case mismatch opening requested file '/sys/class/power_supply/BAT0/capacity', stored as '/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0/capacity' in the filesystem. This file will not open when exported to other case-sensitive platforms.
   At: drivers/unix/file_access_unix.cpp:117.

Those are due to platform/x11/detect_prime.cpp which parses system files to figure out if the system uses switchable graphics.
It seems indeed that a solution that resolves symlinks is also not a good solution then.

EmrysMyrddin · 2019-05-30T16:42:36Z

Yes, i'm agree that this is not a good solution.
But I can't find one which doesn't involve just verifying, folder by folder, that the path has the good case...

Kanabenki · 2019-05-30T18:12:11Z

That was the same conclusion I ended up with ^^

From what I saw while searching for this problem, there isn't an universal way on Unix systems to do that without walking the folder. Still since realpath seems to work as intended on macOS, were the problem is way more likely to happen (because case insensitivity is the default, while at the moment it's not even possible on ext4), it could be kept as a macOS only warning.

EmrysMyrddin · 2019-05-30T18:15:20Z

Following symlinks is probably not a good behaviour ?

Calinou · 2019-05-30T18:15:30Z

@akien-mga Could system paths such as /dev, /proc and /sys be ignored for the purpose of error reporting?

Kanabenki · 2019-05-30T22:29:44Z

@EmrysMyrddin Oh yes you're right, I didn't think about symlink resolution, it certainly bring some problems.

@Calinou But there would still be false positives if the user has symlinks in their project.

akien-mga · 2019-10-30T12:41:42Z

Moving to next milestone for now as this doesn't seem ready to merge yet. Still no idea on how to solve this properly.

aaronfranke · 2020-05-26T14:54:08Z

@EmrysMyrddin Is this still desired? If so, it needs to be rebased on the latest master branch. Even though there are no conflicts, rebasing is important to allow testing how this behaves on master. Also, the above feedback should be addressed.

Otherwise, abandoned pull requests will be closed in the future as announced here.

EmrysMyrddin · 2020-05-26T15:34:27Z

@aaronfranke Yes it's still desired but I'm kind of stuck. I can't figure out how to solve the feedbacks. It seems that there is not clean way to do what we want, since "real path of a file" is not a real unix concept.

I will rebase, but I don't now if there is a solution.

aaronfranke · 2021-01-20T06:44:11Z

@EmrysMyrddin Can you rebase and make another attempt at this? Maybe you will be able to figure it out now. If not, rebasing would still be helpful for anyone wishing to test and/or continue the work of this PR.

EmrysMyrddin · 2024-02-13T16:09:28Z

drivers/unix/file_access_unix.cpp

+		if (real_path != path) {
+			if (real_path.to_lower() != path.to_lower()) {
+				// This is a symlink, some platform may not support them.
+				WARN_PRINT_ONCE_ED("Tried to open a symlink file '" + path + "', referencing to '" + real_path + "' in the filesystem. This file will not open when exported to platforms without symbolic links, like Windows versions before Windows 10.");


I'm using WARN_PRINT_ONCE_ED because for some reasons, it real spam the console with 17 messages each time I save my script ><'

Do you think I should also change the WARN_PRINT to WARN_PRINT_ONCE_ED in the windows version of this check ?

AThousandShips · 2024-02-13T16:11:23Z

Just printing once won't help IMO, it'll be silent after the first print, until you restart the editor

EmrysMyrddin · 2024-02-13T16:22:31Z

It'll be printed once for each file with a case mismatch.
But i can revert to the PRINT_WARN, just the spam is really excessive. I don't know why Godot is opening the file 17 times on each save ><'

AThousandShips · 2024-02-13T16:26:30Z

No it won't be printed once for each, once in total, that's how that macro works, it prints once for each line that has the macro, each invocation

EmrysMyrddin · 2024-02-13T19:57:42Z

I've tested it, and I actually get one warning for each different path with a mismatching name.

See here the result of 2 different macros:

WARN_PRINT_ONCE_ED

WARN_PRINT (I've just opened the project, not even modified the script)

AThousandShips · 2024-02-13T19:59:35Z

Does it happen if you do it in another file? This is very weird as the code shouldn't be doing that in this way, worth investigating on its own:

godot/core/error/error_macros.h

Lines 723 to 731 in e92d55b

    
           #define WARN_PRINT_ONCE_ED(m_msg)                                                                 \ 
        
           	if (true) {                                                                                   \ 
        
           		static bool first_print = true;                                                           \ 
        
           		if (first_print) {                                                                        \ 
        
           			_err_print_error(FUNCTION_STR, __FILE__, __LINE__, m_msg, true, ERR_HANDLER_WARNING); \ 
        
           			first_print = false;                                                                  \ 
        
           		}                                                                                         \ 
        
           	} else                                                                                        \ 
        
           		((void)0)

fire · 2024-02-13T20:18:18Z

Todo:

Resolve opening the file 17 times on each save ><' issue
Team approval
Production Team Approval

EmrysMyrddin · 2024-02-14T14:25:48Z

@AThousandShips You are totally right, I've used my 2 test files, but each are not triggering the same path. One warns about case mismatch, the other warns about symlinks. So yeah, it only warns once... I will revert to the WARN_PRINT then.

@fire I'm not sure if this "open 17 times" thing is in the scope of this PR. Seems like a seperate issue for me, like an optimization issue or something ?

drivers/unix/file_access_unix.cpp

EmrysMyrddin · 2024-03-07T16:12:14Z

I'm removing the warning for symlinks. They are now supported by pretty much all systems (since windows 10).

And I got some people saying they are actually using them to easily share addons between projects.

EmrysMyrddin · 2024-03-07T16:13:22Z

This is now probably ready :-) I think the multiple opening files should be addressed in an other PR if it's not an intended behaviour.

fire · 2024-11-05T19:29:44Z

Since it's ready, can you squash your git commits?

https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html

fire · 2024-11-05T19:31:12Z

@AThousandShips I think there are some code style issues; what do you think?

AThousandShips · 2024-11-05T19:34:25Z

Will do a pass tomorrow!

drivers/unix/file_access_unix.h

drivers/unix/file_access_unix.cpp

fire · 2024-11-06T19:42:53Z

@EmrysMyrddin is it ok if I edit your branch? I can also make a new pr.

fire · 2024-11-07T17:53:09Z

I have made a salvaging pull request #98927

EmrysMyrddin · 2024-11-08T09:51:40Z

Of course yes you can make any edit you need :-)

EmrysMyrddin · 2024-11-08T09:54:33Z

Ho you already made another PR, do you still want me to squash commits and make the required changes ?

When a file is opened with a wrong case, it can work on the developer system but but break on a user system with a casesensitive filesystem. This will display a warning when it happens. CAVEATS : It will also display the warning if a symlink is present somewhere in the path. Fixes godotengine#23441 adapt warning if file is symlink fix memory leak and avoid `lstat` usage do not expose real_path when not in TOOLS_ENABLED mode Update drivers/unix/file_access_unix.cpp Don't warn on symlinks

EmrysMyrddin · 2024-11-08T10:01:56Z

Ok, I've rebased and squash anyway :-) Bu feel free to do whatever is better for you

EmrysMyrddin · 2024-11-16T15:29:06Z

Closed in favor of #98927

EmrysMyrddin force-pushed the warn-file-case-mismatch branch from 575a7c9 to bc964d5 Compare January 9, 2019 09:24

EmrysMyrddin changed the title ~~Warn on case mismatch on macOS~~ Warn on case mismatch Jan 9, 2019

EmrysMyrddin changed the title ~~Warn on case mismatch~~ Warn on case mismatch on Unix systems Jan 9, 2019

EmrysMyrddin mentioned this pull request Jan 9, 2019

Warn on case mismatch while opening a file on macOS #23503

Closed

EmrysMyrddin force-pushed the warn-file-case-mismatch branch from bc964d5 to 22744d0 Compare January 9, 2019 09:36

akien-mga added enhancement platform:linuxbsd platform:macos topic:core labels Jan 9, 2019

akien-mga added this to the 3.2 milestone Jan 9, 2019

akien-mga requested a review from hpvb May 30, 2019 13:24

akien-mga modified the milestones: 3.2, 4.0 Oct 30, 2019

EmrysMyrddin force-pushed the warn-file-case-mismatch branch from 22744d0 to 8f8d256 Compare May 26, 2020 15:35

aaronfranke requested a review from akien-mga June 17, 2020 00:51

EmrysMyrddin commented Feb 13, 2024

View reviewed changes

EmrysMyrddin commented Feb 14, 2024

View reviewed changes

drivers/unix/file_access_unix.cpp Outdated Show resolved Hide resolved

EmrysMyrddin force-pushed the warn-file-case-mismatch branch from 4d2816a to d4ce467 Compare March 7, 2024 16:07

AThousandShips self-requested a review November 5, 2024 19:34

AThousandShips reviewed Nov 6, 2024

View reviewed changes

drivers/unix/file_access_unix.h Show resolved Hide resolved

drivers/unix/file_access_unix.cpp Show resolved Hide resolved

drivers/unix/file_access_unix.cpp Show resolved Hide resolved

fire mentioned this pull request Nov 7, 2024

Warn on filesystem case mismatch #98927

Merged

EmrysMyrddin force-pushed the warn-file-case-mismatch branch from 7b905eb to 78e95d5 Compare November 8, 2024 09:59

EmrysMyrddin closed this Nov 16, 2024

AThousandShips added archived and removed needs work labels Nov 16, 2024

AThousandShips removed this from the 4.x milestone Nov 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn on case mismatch on Unix systems #24860

Warn on case mismatch on Unix systems #24860

EmrysMyrddin commented Jan 9, 2019

EmrysMyrddin commented Jan 9, 2019

EmrysMyrddin commented Jan 9, 2019

Kanabenki commented Jan 10, 2019 •

edited

Loading

EmrysMyrddin commented Jan 10, 2019

akien-mga commented May 30, 2019

akien-mga commented May 30, 2019

EmrysMyrddin commented May 30, 2019

Kanabenki commented May 30, 2019

EmrysMyrddin commented May 30, 2019

Calinou commented May 30, 2019

Kanabenki commented May 30, 2019 •

edited

Loading

akien-mga commented Oct 30, 2019

aaronfranke commented May 26, 2020

EmrysMyrddin commented May 26, 2020

aaronfranke commented Jan 20, 2021

EmrysMyrddin Feb 13, 2024

AThousandShips commented Feb 13, 2024

EmrysMyrddin commented Feb 13, 2024

AThousandShips commented Feb 13, 2024 •

edited

Loading

EmrysMyrddin commented Feb 13, 2024

AThousandShips commented Feb 13, 2024 •

edited

Loading

fire commented Feb 13, 2024

EmrysMyrddin commented Feb 14, 2024

EmrysMyrddin commented Mar 7, 2024

EmrysMyrddin commented Mar 7, 2024

fire commented Nov 5, 2024

fire commented Nov 5, 2024

AThousandShips commented Nov 5, 2024

fire commented Nov 6, 2024

fire commented Nov 7, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 16, 2024

Warn on case mismatch on Unix systems #24860

Warn on case mismatch on Unix systems #24860

Conversation

EmrysMyrddin commented Jan 9, 2019

Purpose

Exemples

Caveats

EmrysMyrddin commented Jan 9, 2019

EmrysMyrddin commented Jan 9, 2019

Kanabenki commented Jan 10, 2019 • edited Loading

EmrysMyrddin commented Jan 10, 2019

akien-mga commented May 30, 2019

akien-mga commented May 30, 2019

EmrysMyrddin commented May 30, 2019

Kanabenki commented May 30, 2019

EmrysMyrddin commented May 30, 2019

Calinou commented May 30, 2019

Kanabenki commented May 30, 2019 • edited Loading

akien-mga commented Oct 30, 2019

aaronfranke commented May 26, 2020

EmrysMyrddin commented May 26, 2020

aaronfranke commented Jan 20, 2021

EmrysMyrddin Feb 13, 2024

Choose a reason for hiding this comment

AThousandShips commented Feb 13, 2024

EmrysMyrddin commented Feb 13, 2024

AThousandShips commented Feb 13, 2024 • edited Loading

EmrysMyrddin commented Feb 13, 2024

AThousandShips commented Feb 13, 2024 • edited Loading

fire commented Feb 13, 2024

EmrysMyrddin commented Feb 14, 2024

EmrysMyrddin commented Mar 7, 2024

EmrysMyrddin commented Mar 7, 2024

fire commented Nov 5, 2024

fire commented Nov 5, 2024

AThousandShips commented Nov 5, 2024

fire commented Nov 6, 2024

fire commented Nov 7, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 8, 2024

EmrysMyrddin commented Nov 16, 2024

Kanabenki commented Jan 10, 2019 •

edited

Loading

Kanabenki commented May 30, 2019 •

edited

Loading

AThousandShips commented Feb 13, 2024 •

edited

Loading

AThousandShips commented Feb 13, 2024 •

edited

Loading