Skip to content

also update $XDG_DATA_DIR (share/) and $GI_TYPELIB_PATH environment variables (lib*/girepository-*)#3133

Merged
boegel merged 1 commit intoeasybuilders:developfrom
Flamefire:GIR_fixes
Dec 23, 2019
Merged

also update $XDG_DATA_DIR (share/) and $GI_TYPELIB_PATH environment variables (lib*/girepository-*)#3133
boegel merged 1 commit intoeasybuilders:developfrom
Flamefire:GIR_fixes

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

Those are required for GIR libs to find *.gir files (usually in /share, XDG_DATA_DIR used)
and *.typelib files (usually in /lib/girepository-1.0, GI_TYPELIB_PATH used)

Adresses easybuilders/easybuild-easyconfigs#9520

Those are required for GIR libs to find *.gir files (usually in /share, XDG_DATA_DIR used)
and *.typelib files (usually in /lib/girepository-1.0, GI_TYPELIB_PATH used)

Adresses easybuilders/easybuild-easyconfigs#9520
'ACLOCAL_PATH': [os.path.join('share', 'aclocal')],
'CLASSPATH': ['*.jar'],
'XDG_DATA_DIRS': ['share'],
'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in lib_paths],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not add to XDG_DATA_DIRS unconditionally. But only when there is a gir-* dir in share.

Copy link
Copy Markdown
Contributor Author

@Flamefire Flamefire Dec 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to make that GIR specific because the docu is more general: https://specifications.freedesktop.org/basedir-spec/0.6/ar01s03.html

$XDG_DATA_DIRS defines the preference-ordered set of base directories to search for data files in addition to the $XDG_DATA_HOME base directory. The directories in $XDG_DATA_DIRS should be seperated with a colon ':'.

If $XDG_DATA_DIRS is either not set or empty, a value equal to /usr/local/share/:/usr/share/ should be used.

This sounds like more than GIR can benefit from having access to the share folders of installed modules.

If this is still unwanted: It will be more complex. I don't think one can change it at this point to add share if share/gir-* exists. Would need to be done at another place then.

Edit: Or we extent the list entries to allow tuples of (<check-expr>, <path-to-add>) where for strings both are equal

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, it just felt a bit too much. But it is what the variable and dir is for so i'll withdraw that comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully understood, I felt the same. But I guess in the end this is better.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a fairly big risk that XDG_DATA_DIRS will end up being VERY long.
There are LOTS of packages that have a share dir.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies to $PATH, $LD_LIBRARY_PATH, etc., I don't think we've run into any serious issues there either because of a long list of entries there...

Copy link
Copy Markdown

@paulmelis paulmelis Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to make a remark on this patch now that is has been applied in newer EB releases and we run into some side-effect of it. This might be relevant for other folks.

The XDG Base Directory Specification mandates:

If $XDG_DATA_DIRS is either not set or empty, a value equal to /usr/local/share/:/usr/share/ should be used.

On many system $XDG_DATA_DIRS is empty (or not set) by default, which means the default directories /usr/local/share/:/usr/share/ are assumed implicitly for searching data files. But when an EB-based module simply uses prepend-path to alter $XDG_DATA_DIRS those directories are no longer implicitly used, as only the value set by the module is present in $XDG_DATA_DIRS. So loading a module has the effect of the global data directories no longer being searched, which has all kinds of side-effects (I ran into this while installing a VNC environment, whose desktop startup failed in strange ways).

I'm not sure this should (or can be easily) fixed on the EB side. One workaround is to set $XDG_DATA_DIRS=/usr/local/share/:/usr/share/ in the system shell skeleton files, for example. But many other system scripts I found that alter $XDG_DATA_DIRS make sure to add the /usr/local/share/:/usr/share/ entries when the current value for $XDG_DATA_DIRS is empty.

All in all the way the value is mandated in the specification is a bit troubled, due these kinds of nasty side-effects.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not enforced, the text above uses "should" not "must". But yes, it does hint at problems...

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine, it's not mandatory, but then there need to be good reasons to deviate. Plus the real world does assume the stated principle in many cases (either explicitly or implicitly).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up in #3368 on this issue...

@boegel boegel added this to the release after 4.1.0 (4.1.1?) milestone Dec 23, 2019
@boegel boegel changed the title Add XDG_DATA_DIR and GI_TYPELIB_PATH env vars also update $XDG_DATA_DIR (share/) and $GI_TYPELIB_PATH environment variables (lib*/girepository-*) Dec 23, 2019
@boegel boegel changed the title also update $XDG_DATA_DIR (share/) and $GI_TYPELIB_PATH environment variables (lib*/girepository-*) also update $XDG_DATA_DIR (share/) and $GI_TYPELIB_PATH environment variables (lib*/girepository-*) Dec 23, 2019
@boegel boegel merged commit ac02b67 into easybuilders:develop Dec 23, 2019
@Flamefire Flamefire deleted the GIR_fixes branch December 23, 2019 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants