Skip to content

Conversation

@AndiH
Copy link
Contributor

@AndiH AndiH commented Jul 25, 2018

Also fixes #6602 of easybuilders/easybuild-easyconfigs (easybuilders/easybuild-easyconfigs#6602)

guesses.update({
'PATH': bin_path,
'LD_LIBRARY_PATH': lib_path,
'LIBRARY_PATH': ['lib64/stubs'],
Copy link
Member

Choose a reason for hiding this comment

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

This makes LIBRARY_PATH to just check for lib64/stubs. A possible work around is:

'LIBRARY_PATH': lib_path+['lib64/stubs'],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LIBRARY_PATH wasn't set at all before, so I did not want to change too much. Two notes to regarding your suggestion:

  1. lib_path+['lib64/stubs']: There's a lib64 too much!
  2. lib_path: That would also include extras/CUPTI/lib64 and nvvm/lib64 resulting in lib64/stubs being added there…

Copy link
Member

Choose a reason for hiding this comment

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

Mmmmh, I see LIBRARY_PATH set to pathJoin(root, "lib64") in the module, when installing it without this fix. Where did you see it as not set at all? Or did you meant that it was not explicitly updated in guesses.update? If the latter, it wasn't explicitly mentioned here, but if you update the dictionary you are overwriting the old definition of that key, which was indeed defined.

Regarding 1.: I am not sure what you mean :-?
Regarding 2.: That's correct. Maybe just change it to 'LIBRARY_PATH': ['lib64', 'lib64/stubs'],?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably don't know enough about EasyBuild. Indeed I thought that guesses.update() would be the only definition of LIBRARY_PATH.

  1. Oh! I was stupied. I forgot that lib_path is a list and + just appends the list. For some reason I thought of the + more as a cross product, adding lib64/stubs to every entry of lib_path.
  2. Ok, fine with me!

@boegel boegel added this to the 3.6.3 milestone Aug 16, 2018
@damianam
Copy link
Member

@akesandgren anything to add before we merge this? I kind of need it in for another PR later on.

@akesandgren
Copy link
Contributor

You should use os.join.path() instead of explicitly using "a/b"
That's all as far as I can see.

@boegel
Copy link
Member

boegel commented Aug 16, 2018

I'll let @akesandgren's remark slip, since the added lines just follow the style of the preceding ones.

@damianam Can you look into cleaning that up in your follow-up PR?

@damianam
Copy link
Member

That's a good point, but there are other parts of the easyblock that would need to be changed, regardless of the purpose of the PR. I'll merge the PR now, since functionality wise it is all good and Andreas is on holidays. Then I'll make another PR myself to address that suggestion and do something extra I need.

@damianam damianam merged commit 2e1116c into easybuilders:develop Aug 16, 2018
@AndiH AndiH deleted the 1463_6602_cuda_stubs branch September 17, 2018 12:10
@mboisson
Copy link
Contributor

@boegel, sorry to do thread necropsy, but just as a note, adding stubs to LIBRARY_PATH is causing us problems, because it ends up in our binaries RUNPATH.

If this happens and you don't use LD_LIBRARY_PATH (because you are using RUNPATH), you will have problems at runtime.

This is probably something you need to check when you are using runpath/rpath with easybuild (for Compute Canada, @bartoldeman will have to adjust our linker wrapper or we need to not put the stubs in our LIBRARY_PATH)

A stub object is a shared object, built entirely from mapfiles, that supplies the same linking interface as the real object, while containing no code or data. Stub objects cannot be used at runtime.

@bartoldeman
Copy link
Contributor

I adjusted our linker wrapper to not convert -L.../stubs into -rpath .../stubs now.

I guess the same could be done to https://github.com/easybuilders/easybuild-framework/blob/develop/easybuild/scripts/rpath_args.py which does similar things but I haven't tested EB's rpath support yet.

@boegel
Copy link
Member

boegel commented Dec 6, 2018

@mboisson @bartoldeman Please open an issue in framework to get this fixed in our RPATH support, indeed in rpath_args.py

@mboisson
Copy link
Contributor

mboisson commented Dec 6, 2018

done

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.

6 participants