eckit icon indicating copy to clipboard operation
eckit copied to clipboard

Add support for colon seperated list of library homes

Open ChrisspyB opened this issue 1 year ago • 1 comments

Allows setting multiple homes in a colon seperated string. e.g. ECKIT_HOME=/path/to/home1:/path/to/home2 This is useful for libraries which like to search for multiple configs, which may be in distinct directories. LocalPathName's tilde expansion ~eckit/file will match the first existing file from [/path/to/home1/file1, /path/to/home2/file]. If none exist, the first is used.

No change in behaviour is intended if a single home is set.

If LIB_HOME is not set, there is a change in behaviour in the tilde expansion:

  • Before: eckit would search recursively upwards from the prefix directory all the way to the root directory.
    • e.g. ~eckit/file1 -> libraryprefix/file1, libraryprefix/../file1, libraryprefix/../../file1, libraryprefix/../../../file1... /file1
  • Now: eckit will search the prefix directories and one and two levels above it, then go straight to the root. (exactly four directories)
    • e.g. ~eckit/file1 -> libraryprefix/file1, libraryprefix/../file1, libraryprefix/../../file1, libraryprefix/../../file1, /file1

note This does mean that LIB_HOME cannot be set to directories whose path contains a colon.

ChrisspyB avatar Sep 20 '24 16:09 ChrisspyB

Codecov Report

Attention: Patch coverage is 93.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.73%. Comparing base (3598aa9) to head (beff400). Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
src/eckit/system/Library.cc 87.50% 3 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #140   +/-   ##
========================================
  Coverage    63.72%   63.73%           
========================================
  Files         1065     1065           
  Lines        55141    55161   +20     
  Branches      4084     4085    +1     
========================================
+ Hits         35139    35157   +18     
- Misses       20002    20004    +2     

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

codecov-commenter avatar Sep 20 '24 16:09 codecov-commenter

Hello past me. I think this would be nice to have. I should re-review what I wrote a year ago and then push someone else to review/merge.

ChrisspyB avatar Aug 15 '25 14:08 ChrisspyB

Hi @ChrisspyB , while I'm an outsider here, there are a couple of questions open by this feature:

  • what happens if configuration files of the same name are found in multiple paths (ie. is there a precedence)
  • are non-existant paths allowed
  • is this the same behaviour you get from a bundle, ie., separately PROJECT_A_HOME=/this and PROJECT_B_HOME=/that, then PROJECTS_A_B_HOME=/this:/that - (1) do these work the same way (I believe additional rules are required for this), and (2) does precendence (?) also work the same?

Maybe there's already an answer for these but I couldn't find it in the tests (I skipped the code)

pmaciel avatar Aug 18 '25 08:08 pmaciel

Hi @pmaciel. I will have to review this myself too as it was a long time ago, but from what I remember.

what happens if configuration files of the same name are found in multiple paths

The intention is the first found path takes precedence. The precedence for lookups is HOME=$FIRST_PATH:$SECOND_PATH

are non-existant paths allowed

Yes, as these are allowed with the current implementation. In these cases we resolve to the first home. e..g the ~home/not_existent_file will resolve to /path/to/first_home/non_existent_file, if the file is not present in any of the home directories.

ie., separately PROJECT_A_HOME=/this and PROJECT_B_HOME=/that, then PROJECTS_A_B_HOME=/this:/that

I was completely unaware of this functionality, and it seems very likely that these changes would break it... (e.g. what if I set 2 homes for A and 2 homes for B...?).

ChrisspyB avatar Aug 19 '25 10:08 ChrisspyB