Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config wildcard handling script #901

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

TheTesla
Copy link

The recommended approach handling wildcards in config file with bash script, done for systemd.

@bluca
Copy link
Member

bluca commented Jul 22, 2017

Could you please make this optional behind a --configure flag? It's a workaround for a broken distro, so it should not be the default

@TheTesla
Copy link
Author

How should this config flag work? Should it take action at build time? Should this be a flag oft bumblebee.bash at run time?

@bluca
Copy link
Member

bluca commented Jul 22, 2017

A build time flag to control what is written in the systemd service file would be enough. This way the ExecStart will be generated to call that bash script only if the flag is true.

Copy link
Member

@Lekensteyn Lekensteyn left a comment

Choose a reason for hiding this comment

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

What distro was this needed for? In the past we had a similar hack for Ubuntu packages:
https://github.com/Bumblebee-Project/bumblebee-ppa/blob/master/xenial/bumblebee/debian/bumblebee.bumblebeed.upstart

@@ -101,6 +101,17 @@ AS_IF([test "x$with_pidfile" != xno],
])
AM_CONDITIONAL([WITH_PIDFILE], [test "x$with_pidfile" != xno])


AC_ARG_ENABLE([wild],
[ --enable-wild Turn on wildcard support],
Copy link
Member

Choose a reason for hiding this comment

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

A "wild" daemon is probably not a good idea ;)

Maybe the option and description could be changed to more accurately reflect its functionality?

./configure CONF_DRIVER=nvidia CONF_DRIVER_MODULE_NVIDIA=nvidia-375 \
CONF_PRIMUS_LD_PATH=/usr/lib/x86_64-linux-gnu/primus:/usr/lib/i386-linux-gnu/primus \
CONF_LDPATH_NVIDIA=/usr/lib/nvidia-375:/usr/lib32/nvidia-375 \
CONF_MODPATH_NVIDIA=/usr/lib/nvidia-375/xorg,/usr/lib/xorg/modules \
Copy link
Member

Choose a reason for hiding this comment

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

This is prone to getting outdated. Remove it? Instructions in the README should be clear


ConfDriver=$( crudini --get $ConfFile bumblebeed driver )
ConfKernelDriver=$( crudini --get $ConfFile driver-nvidia kerneldriver )
ConfLibraryPath=$( crudini --get $ConfFile driver-nvidia librarypath )
Copy link
Member

Choose a reason for hiding this comment

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

crudini is not installed by default.

@Lekensteyn
Copy link
Member

I have forgotten about the earlier PR (#889), but it looks like this does not properly handle multiple installed versions. The glob pattern simply adds all matches.

What about this simpler alternative that does not other dependencies:
https://github.com/Bumblebee-Project/bumblebee-ppa/blob/master/xenial/bumblebee/debian/bumblebee.bumblebeed.upstart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants