fangohr / octopus-in-docker

Dockerfile for Octopus. Useful for tutorials, OSX, Windows, testing.
BSD 3-Clause "New" or "Revised" License
0 stars 3 forks source link

Demo workaround for issue 9 - do not merge. #8

Closed fangohr closed 7 months ago

fangohr commented 10 months ago

…issues/7)

iamashwin99 commented 10 months ago

The fix in this MR is not the solution, as we are compiling octopus spglib (@2.0.2), but linking against system spglib (@debian-1.16.1) see https://github.com/fangohr/octopus-in-docker/issues/9#issuecomment-1864135871

fangohr commented 10 months ago

The fix in this MR is not the solution, as we are compiling octopus spglib (@2.0.2), but linking against system spglib (@debian-1.16.1) see #9 (comment)

That's what I tried initially (first few commits that added the debian package for spglib). I then undid those commits (removed installing spglib). I then found out that by setting the LD_LIBRARY_PATH we can make Octopus use the /usr/local/lib/libsymspg.so.0 that it has compiled before.

So if you look at the changes in this PR, we are correctly linking to the spglib that Octopus has built (and do not install a Debian package with spglib).

(It would be better to fix the autotools/installation of spglib so that the libsysmpsg.so can be found without setting the LD_LIBRARY_PATH of course.)

iamashwin99 commented 10 months ago

My bad, I overlooked the path in

2023-12-20T05:50:24.4096397Z #26 [21/32] RUN ldd /usr/local/bin/octopus | grep libsym
2023-12-20T05:50:24.5127927Z #26 0.211  libsymspg.so.0 => /usr/local/lib/libsymspg.so.0 (0x00007f68b5a7a000)
2023-12-20T05:50:24.6634561Z #26 DONE 0.2s

and imagined that it was coming from a debian installed package, even though in the octopus compilation we also compile spglib.

Looks like spglib was always used from the "internally shipped" spglib. Im not sure why the new version fails to be linked while the old one used to be added automatically. Ill investigate this later.

For now this looks good and can be merged

fangohr commented 10 months ago

Thanks for looking into this, Ashwin, and the approval.

I would prefer if the problem was fixed at the source in Octopus. It seems to be related to the changes of the spglib source distributed with Octopus (see https://github.com/fangohr/octopus-in-docker/issues/9#issuecomment-1867342371)

fangohr commented 10 months ago

As I soon plan to include the work-around shown here (LD_LIBRARY_PATH) in the main branch of this repo (only until the bug is fixed upstream in Octopus), I have just created a dedicated branch (fix-libsymspg-error_Snapshot-of-main-for-lasting-diff_Remove-when-issue-9-is-resolved) to preserve the current state of main, so that the "Changes" for this merge request show the relevant lines. I have also updated the base branch for this MR to the new branch (fix-libsymspg-error_Snapshot-of-main-for-lasting-diff_Remove-when-issue-9-is-resolved). As of now, this is exactly the same as main (but that will change later).

When this issue is resolved, we can delete the branch fix-libsymspg-error_Snapshot-of-main-for-lasting-diff_Remove-when-issue-9-is-resolved.

This MR should not be merged, but rather be used to remember what changes in main need to be undone when the bug is fixed (i.e. remove setting of LD_LIBRARY_PATH).

fangohr commented 7 months ago

Closed with #26