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

NSBundle loads resources from the wrong version #351

Closed
rmottola opened this issue Nov 21, 2023 · 18 comments
Closed

NSBundle loads resources from the wrong version #351

rmottola opened this issue Nov 21, 2023 · 18 comments

Comments

@rmottola
Copy link
Member

When asked to load resources from gui (class NSApplication) if e.g. gui version is 1.30 and there are several versions in the resources dir, the wrong one gets loaded.
E.g. with current 0.30 the resources are looked on 0.28 if present.

@rmottola
Copy link
Member Author

On windows, I se this error, which is new.
It appears that this issue has many aspects.

2023-11-21 12:46:36.327 DataBasin[15152:9020] Warning: [NSBundle+bundleForLibrary:version:] called without version and unable to infer version from library name (C:\msys64\mingw64\Local\Applications\DataBasin.app\DataBasin.exe).

@rfm
Copy link
Contributor

rfm commented Nov 21, 2023 via email

@rfm
Copy link
Contributor

rfm commented Nov 21, 2023

That particular error seems to be windows-specific. The code was comparing two paths (location of class definition and the executable) to see if they were the same (ie the class was defined in the executable rather than a library). The comparison said the two were different, but that was because the path separators were slashes in one and backslashes in the other.
I fixed that by changing the code generating the symbol path to standardise the returned path, the same way that the code generating the executable path does.

@rfm
Copy link
Contributor

rfm commented Nov 21, 2023

FYI [NSBundle+bundleForLibrary:version:] tries to infer the version from the library name/path if it is not provided as an argument. So if it's loading the wrong version that's because
a. no version argument was provided and
b. no version could be inferred from the library argument
I made a change to the GSPrivateSymbolPath() function to resolve symbolic links in the path it returns. This should ensure that, if the library name is provided by that function, it will contains the correct information to infer library version in cases like where the library is libfoo.so and that is a link to a file with version numbers such as libfoo.1.1.so

@gcasa
Copy link
Member

gcasa commented Nov 22, 2023

From looking at the code in loadNibNamed:... it doesn't appear as though any of those methods call bundleForLibrary:version: or anything that might do a library specific search. They assume that bundleForClass: will return the right path. I'm not in front of my computer at the moment so I could be mistaken, but I don't think it does. This might be part of our issue.

@rfm
Copy link
Contributor

rfm commented Nov 22, 2023

They assume that bundleForClass: will return the right path.
The bundleForClass: method depends on bundleForLibrary:version
Essentially the algorithm is
If we have a cached bundle for the class (eg from a framework), return that.
Get the path of the file containing the class object code
If the same as the path of the executable, return the bundle of the executable
Otherwise (the class must be in a library), return the bundle of the library at that path (inferring version from path)

@rmottola
Copy link
Member Author

I just checked and the code seems to solve my issue, bundle resource now loads.
My test is just running latest version and gets the gorm file from latest version.
A more complicated case would be to check that running an older version also gets the corresponding older version resource, but such a case is a little bit more complicated to set up.
Or we can trust the code now :)

@rmottola
Copy link
Member Author

I get dozens of messages when running GWorkspace:

2023-11-27 17:26:06.983 GWorkspace[32433:32433] Warning: [NSBundle+bundleForLibrary:version:] called without version and unable to infer version from library name (/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace).
2023-11-27 17:26:07.015 GWorkspace[32433:32433] Warning: [NSBundle+bundleForLibrary:version:] called without version and unable to infer version from library name (/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace).
2023-11-27 17:26:07.028 GWorkspace[32433:32433] Warning: [NSBundle+bundleForLibrary:version:] called without version and unable to infer version from library name (/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace).
2023-11-27 17:26:07.032 GWorkspace[32433:32433] Warning: [NSBundle+bundleForLibrary:version:] called without version and unable to infer version from library name (/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace).
...
Must investigate more what is happening here...

@rfm
Copy link
Contributor

rfm commented Nov 27, 2023

unable to infer version from library name
That message no longer exists: I would recommend updating base from git.

@rmottola
Copy link
Member Author

unable to infer version from library name
That message no longer exists: I would recommend updating base from git.

yes sorry, a second clean&rebuild of base fixed that... strange

@rmottola
Copy link
Member Author

Things improved, but I still have warnings when e.g. GWorkspace is started but the CWD is its build dir (e.g. app-gworkspace) but all is fine if the CWD is e.g. just the home directory.

2023-12-13 23:42:20.147 GWorkspace[25140:25140] Warning: [NSBundle+bundleForClass:] unable to determine version of library '/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace' containing 'GWorkspace' for executable '/System/Applications/GWorkspace.app/GWorkspace'

@rfm
Copy link
Contributor

rfm commented Dec 14, 2023

I think you need to look into where/how the wrong path is being found.
The two paths come from
GSPrivateExecutablePath() ... should be the path of the running process
GSPrivateSymbolPath() ... should be the path of the find in which a class is implemented (dynamic library or executable)

So presumably one of those functions return the wrong result in some cases

@rmottola
Copy link
Member Author

@rfm I added two logs at line 1163. I get when launching from the GIT repo dir:

2023-12-19 12:01:49.027 GWorkspace[5783:5783] exec path: /System/Applications/GWorkspace.app/GWorkspace
2023-12-19 12:01:49.028 GWorkspace[5783:5783] private symbol path. /home/multix/gnustep-cvs/apps-gworkspace/GWorkspace
2023-12-19 12:01:49.028 GWorkspace[5783:5783] Warning: [NSBundle+bundleForClass:] unable to determine version of library '/home/multix/gnustep-cvs/apps-gworkspace/GWorkspace' containing 'StartAppWin' for executable '/System/Applications/GWorkspace.app/GWorkspace'

The running process is correct, the private symbol path is wrong

If I launch GWorkspace from my home, I get:

2023-12-19 12:30:31.519 GWorkspace[752:752] exec path: /System/Applications/GWorkspace.app/GWorkspace
2023-12-19 12:30:31.519 GWorkspace[752:752] private symbol path. GWorkspace

so the latter is then relative? Do you expect GSPrivateSymbolPath to be an absolute path?

@rfm
Copy link
Contributor

rfm commented Dec 19, 2023 via email

@rmottola
Copy link
Member Author

Ok, now the warning seems to be gone, I see now:

2023-12-19 15:14:35.312 GWorkspace[22325:22325] exec path: /System/Applications/GWorkspace.app/GWorkspace
2023-12-19 15:14:35.312 GWorkspace[22325:22325] private symbol path. /System/Applications/GWorkspace.app/GWorkspace

or

2023-12-19 21:53:28.174 GWorkspace[26709:26709] exec path: /System/Applications/GWorkspace.app/GWorkspace
2023-12-19 21:53:28.174 GWorkspace[26709:26709] private symbol path. /System/Applications/GWorkspace.app/GWorkspace

which looks the same, independently if I started in my home our source dir and the warning is gone. Seems fixed!

I think this needs to be tested a bit!

@rfm
Copy link
Contributor

rfm commented Jan 19, 2024

Do you think it's OK to close this issue now?

@rfm
Copy link
Contributor

rfm commented Feb 9, 2024

I think we are OK on this one now.

@rfm rfm closed this as completed Feb 9, 2024
@rmottola
Copy link
Member Author

Yes, I confirm... all originally discovered issues were solved.

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

No branches or pull requests

3 participants