Skip to content
This repository has been archived by the owner on Jan 2, 2024. It is now read-only.

Spicy-plugin creates dangling links (assuming binary installation) #119

Closed
0xxon opened this issue Jun 15, 2022 · 8 comments · Fixed by #121
Closed

Spicy-plugin creates dangling links (assuming binary installation) #119

0xxon opened this issue Jun 15, 2022 · 8 comments · Fixed by #121
Assignees

Comments

@0xxon
Copy link
Member

0xxon commented Jun 15, 2022

An installation of spicy-plugin will leave the following dangling link in include/zeek/builtin-plugins/spicy-plugin/lib64:

[johanna@johanna-zeek-compile lib64]$ ls -lh
total 1.0K
lrwxrwxrwx 1 johanna johanna 57 Jun 15 09:57 zeek-spicy -> /home/johanna/zeek/build/src/builtin-plugins/spicy-plugin

Assuming a binary installation, this link will point into nothingness; having links to outside of the distribution directory also violates packaging guidelines on just about any system, and generally seems like a bad idea.

@bbannier pointed me to this as the cause

spicy-plugin/CMakeLists.txt

Lines 211 to 216 in 8cd9a0b

if (ZEEK_SPICY_PLUGIN_INTERNAL_BUILD)
# The static build doesn't put the aux files into the build directory
# We create links to get the same effect.
file(MAKE_DIRECTORY "${PROJECT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}")
file(CREATE_LINK "${PROJECT_BINARY_DIR}"
"${PROJECT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}/zeek-spicy" SYMBOLIC)
and mentioned that there might have been a discussion between @sethhall and @rsmmr about this.

I am not sure what the reason for this is - but in my opinion we should not have links back into the source folder in a finished installation.

Optimally this should be removed for the 5.0 release.

@rsmmr
Copy link
Member

rsmmr commented Jun 15, 2022

I don't quite recall the context here, but I'll take a look.

@rsmmr rsmmr self-assigned this Jun 15, 2022
@rsmmr
Copy link
Member

rsmmr commented Jun 17, 2022

Does "an installation of spicy-plugin" refer to a doing Zeek installation with Spicy being built-in? I assume so because that CMake code addresses that case (ZEEK_SPICY_PLUGIN_INTERNAL_BUILD). However, I don't think that CREATE_LINK should be affecting anything at install time. And I don't see any dangling link after make install with current Zeek master.

Asked differently: how to reproduce this? how are you building/installing things?

@rsmmr
Copy link
Member

rsmmr commented Jun 21, 2022

@0xxon ☝️

@0xxon
Copy link
Member Author

0xxon commented Jun 22, 2022

Sorry for the delay. Steps to reproduce with current master on Fedora 36:

./configure --binary-package --enable-static-broker --enable-static-binpac --disable-broker-tests --build-type=Release --prefix=$HOME/install --generator=Ninja
cd build
ninja install
ls -lh $HOME/install/include/zeek/builtin-plugins/spicy-plugin/lib64
total 1.0K
lrwxrwxrwx 1 johanna johanna 57 Jun 22 15:00 zeek-spicy -> /home/johanna/zeek/build/src/builtin-plugins/spicy-plugin

@rsmmr
Copy link
Member

rsmmr commented Jun 24, 2022

I still don't see a lib64 link at all, tried it on Ubuntu. I'll try F36 next.

# ./configure --binary-package --enable-static-broker --enable-static-binpac --disable-broker-tests --build-type=Release --prefix=/home/robin/tmp/install --generator=Ninja --build-dir=build-ubuntu
# cd build-ubuntu
# ninja install
# ls -al $HOME/tmp/install/include/zeek/builtin-plugins/spicy-plugin
total 48
drwxr-xr-x 9 robin robin 4096 Jun 24 09:50 .
drwxr-xr-x 3 robin robin 4096 Jun 24 09:50 ..
drwxr-xr-x 2 robin robin 4096 Jun 24 09:50 bin
drwxr-xr-x 2 robin robin 4096 Jun 24 09:50 cmake
-rw-r--r-- 1 robin robin 1579 Jun 23 13:32 consts.bif.h
-rw-r--r-- 1 robin robin  820 Jun 23 13:32 events.bif.h
-rw-r--r-- 1 robin robin  696 Jun 23 13:32 functions.bif.h
drwxr-xr-x 3 robin robin 4096 Jun 24 09:50 include
drwxr-xr-x 2 robin robin 4096 Jun 24 09:50 lib
drwxr-xr-x 2 robin robin 4096 Jun 24 09:50 spicy
drwxr-xr-x 3 robin robin 4096 Jun 24 09:50 src
drwxr-xr-x 3 robin robin 4096 Jun 24 09:50 tests

@0xxon
Copy link
Member Author

0xxon commented Jun 24, 2022

Well, on ubuntu the same broken link is in the lib subdirectory, instead of lib64 :)

(at least for me with those options on Ubuntu 22.04)

@rsmmr
Copy link
Member

rsmmr commented Jun 27, 2022

Well, on ubuntu the same broken link is in the lib subdirectory, instead of lib64 :)

I actually didn't see any link even with that configure, not sure why. Either way, looks like Benjamin found a solution.

@0xxon
Copy link
Member Author

0xxon commented Jun 27, 2022

Great, thank you.
Out of curiosity - this only removes the symlinks in binary packaging mode. Which means that for most people that do source installations, they will still be present - even if the source-tree is removed later. Why are the links needed at all?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants