Re: [Buildroot] [PATCH v2] package/fluent-bit: add new package

[Peter Korsgaard <peter@korsgaard.com>]

>>>>> "Thomas" == Thomas Devoogdt <thomas@devoogdt.com> writes:

Hello,

 >> Now we just miss the change summary ;)

 > Sorry about that. I'm not comfortable using git-mail. I at least won't
 > be sad if buildroot goes to e.g. GitHub or GitLab.
 > But that is no excuse.

;)

We do have some documentation on
https://buildroot.org/downloads/manual/manual.html#submitting-patches,
but the summary is simply just to run git format-patch to generate the
patch file, edit it to add the information about the change and then run
git send-email on the patch file instead of directly running git
send-email to generate patch + send it in one go.

>> > +stop() {
 >> > +    printf "Stopping fluent-bit: "
 >> > +    start-stop-daemon -K -q -p $PID_FILE
 >> > +    # https://docs.fluentbit.io/manual/administration/configuring-fluent-bit/yaml/configuration-file#config_section
 >> > +    # The default grace time is set to 5 seconds, so use 6 seconds to have some margin.
 >> > +    TIMEOUT=6
 >> > +    PID=$(cat $PID_FILE 2>/dev/null)
 >> > +    while kill -0 $PID 2>/dev/null; do
 >> > +            [ $TIMEOUT -eq 0 ] && echo "FAIL" && return 1
 >> > +            TIMEOUT=$((TIMEOUT - 1))
 >> > +            sleep 1
 >> > +    done
 >> > +    rm -f $PID_FILE
 >> 
 >> So the pidfile is not removed on timeouts. Is that on purpose?

 > No idea what is preferably, I see at least that it is also not removed in
 > https://nightly.buildroot.org/#adding-packages-start-script.

My point was that you were explictly removing the pid file in the happy
case (which is fine), but not in the unhappy one. Perhaps just move the
rm line just above the while?

 >> I do see that it gets confused and ends up installing the default config
 >> files in usr/etc/fluent-bit. I don't right away see why this is so, but
 >> indeed explicitly setting it to /etc fixes that. Notice that it does
 >> lead to an invalid service file:
 >> 
 >> https://github.com/fluent/fluent-bit/issues/6619
 >> 
 >> So maybe a better fix is to just move the files in a post-install hook?

 > I do prefer to get it done by setting CMAKE_INSTALL_SYSCONFDIR.
 > But perhaps, we could wait the response on that thread?

Ok. Perhaps add a reference to that bugreport in a comment?


 >> > +define FLUENT_BIT_POST_INSTALL_TARGET_HOOK
 >> > +    cp -dpf $(@D)/lib/libminiz*.so* $(TARGET_DIR)/usr/lib/
 >> > +endef
 >> > +
 >> > +FLUENT_BIT_POST_INSTALL_TARGET_HOOKS += FLUENT_BIT_POST_INSTALL_TARGET_HOOK
 >> 
 >> Again, why not just build with -DBUILD_SHARED_LIBS=OFF?
 >> 
 >> I see that the package installs a shared library in a custom
 >> location:
 >> 
 >> target/usr/lib/fluent-bit/libfluent-bit.so
 >> 
 >> What is that used for / how does it find it? I see there is a
 >> FLB_SHARED_LIB option to disable it.

 > Already changed that in V3.
 > https://patchwork.ozlabs.org/project/buildroot/patch/20230123151459.1899697-1-thomas.devoogdt@barco.com/

I guess that is a reference to the miniz issue rather than the
FLB_SHARED_LIB one? Why do you need to patch the sources for that
instead of just passing -DBUILD_SHARED_LIBS=OFF in FLUENT_BIT_CONF_OPTS?

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot