gnudatalanguage / gdl

GDL - GNU Data Language
GNU General Public License v2.0
274 stars 61 forks source link

Add udunits xmls to windows distribution #1765

Closed pjb7687 closed 5 months ago

pjb7687 commented 5 months ago

Should fix #1732, #1723

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 42.72%. Comparing base (20545ef) to head (4f41cb3).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1765 +/- ## ======================================= Coverage 42.72% 42.72% ======================================= Files 361 361 Lines 97320 97320 Branches 19918 19918 ======================================= Hits 41576 41576 Misses 55744 55744 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pjb7687 commented 5 months ago

image

Seems to be fixed with this.

slayoo commented 5 months ago

@pjb7687 how could we check it on CI if that works?

slayoo commented 5 months ago

(and first, that it fails with current master code)

GillesDuvert commented 5 months ago

@pjb7687 Thanks, relevant files are in the installer . Not obvious how to check if they will be found by the executable when installed on a virgin Windows, though (need a specific PATH?). BTW, it should be the same problem on OSX, the DMG does not have those files.

pjb7687 commented 5 months ago

@pjb7687 how could we check it on CI if that works?

I tried to write a pro test code to detect this but weirdly GDL gives exit code 0 when the error occurs, and the error cannot be detected. I am trying out other options I will keep you posted when I make some progress on it

pjb7687 commented 5 months ago

@pjb7687 Thanks, relevant files are in the installer . Not obvious how to check if they will be found by the executable when installed on a virgin Windows, though (need a specific PATH?). BTW, it should be the same problem on OSX, the DMG does not have those files.

It is easy to ship the XML files within the DMG, but it is super tricky for the dylibs to point that xmls within the package. On Mac, all the paths are absolute in the dylib and initially I had to patch their rpath (it seems gdl now uses dylibbundler instead which looks nice, but I assume it does basically the same thing). The problem is that in the dylibs, the paths to resources are still absolute, and this won't be easy to patch. Actually that was the main reason I was hesitant to merge the DMG PR... This is not only a problem with udunits but all other dylibs rely on external resources.

pjb7687 commented 5 months ago

Sorry guys for my delayed answer. I recently had a budget cut by the Korean government (https://www.nature.com/articles/d41586-024-00525-7) and have been super busy writing grants to secure money for my students. The progress would still be very slow, but I will keep you posted once this is done.

And I think I am going to release 1.0.5 after merging this.

GillesDuvert commented 5 months ago

@pjb7687 Thanks, relevant files are in the installer . Not obvious how to check if they will be found by the executable when installed on a virgin Windows, though (need a specific PATH?). BTW, it should be the same problem on OSX, the DMG does not have those files.

It is easy to ship the XML files within the DMG, but it is super tricky for the dylibs to point that xmls within the package. On Mac, all the paths are absolute in the dylib and initially I had to patch their rpath (it seems gdl now uses dylibbundler instead which looks nice, but I assume it does basically the same thing). The problem is that in the dylibs, the paths to resources are still absolute, and this won't be easy to patch. Actually that was the main reason I was hesitant to merge the DMG PR... This is not only a problem with udunits but all other dylibs rely on external resources.

I had to switch to dylibbundler at some point