Page MenuHomePhabricator

efreet: Add installation prefix search for XDG_DATA_DIRS.
ClosedPublic

Authored by netstar on Jan 3 2019, 5:11 AM.

Details

Summary

Currently path parsed for XDG_DATA_DIRS is hard-coded to
/etc. By using eina_prefix_get and adding to the list
of directories efreet should use efreet will use path
relative to the EFL installation.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
netstar created this revision.Jan 3 2019, 5:11 AM
netstar requested review of this revision.Jan 3 2019, 5:11 AM
netstar updated this revision to Diff 18195.Jan 3 2019, 5:13 AM

Fix unused variable warning.

netstar updated this revision to Diff 18196.Jan 3 2019, 5:15 AM

Add test for directory existence.

I can clearly see the intention of this patch, however, the XDG base directory specification clearly states:

If $XDG_CONFIG_DIRS is either not set or empty, a value equal to /etc/xdg should be used.

(https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)

I am just not sure we have a guideline there, btw. who is installing in <prefix>/etc/ ?

zmike added a subscriber: zmike.Jan 3 2019, 6:43 AM

This is a tough call. The variable here does not technically represent the XDG_CONFIG_DIRS variable, it represents efreet's interpretation of that variable as needed by EFL to function.

I think that a safer method for this would be to change this to use the buildsystem sysconfdir variable here; the prefix of this dir is not always the same as the prefix for binaries and libraries, so this should be safer.

Thoughts?

netstar updated this revision to Diff 18198.Jan 3 2019, 7:01 AM

Use safer PACKAGE_SYSCONF_DIR over eina_prefix*

netstar updated this revision to Diff 18199.Jan 3 2019, 7:03 AM

Brain netstar. Use.

zmike requested changes to this revision.EditedJan 3 2019, 9:41 AM

Hold up, this should definitely check to make sure it isn't adding a duplicate PACKAGE_SYSCONF_DIR/xdg entry to the list (ideally before the ecore_file_exists check).

This revision now requires changes to proceed.Jan 3 2019, 9:41 AM
netstar updated this revision to Diff 18205.Jan 3 2019, 10:50 AM

Check for existence.

netstar updated this revision to Diff 18207.Jan 3 2019, 10:53 AM

Dont leak netstar!

zmike requested changes to this revision.Jan 3 2019, 10:59 AM

Looks good, fix formatting and I'll stamp it.

src/lib/efreet/efreet_base.c
310

nit: need () around the first part of this statement to comply with coding style.

This revision now requires changes to proceed.Jan 3 2019, 10:59 AM
zmike accepted this revision.Jan 3 2019, 11:10 AM
This revision is now accepted and ready to land.Jan 3 2019, 11:10 AM
This revision was automatically updated to reflect the committed changes.