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

allow sysctl path to be configured at build time #428

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

aanderse
Copy link
Contributor

my sysctl isn't located in /sbin - i followed convention seen elsewhere

Copy link
Owner

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Great idea to make this configurable! You'll undoubtedly find more.

The format of these have been a bit up in the air. For internal ones, and those sourced from paths.h for instance, they are usually prefixed with a _PATH, but for those read at configure time I think we should go with FOO_PATH.

Example (which needs updating): modules-load.c use MODPROBE_PATH, which needs an #ifndef like yours in procps.c.

So please go for SYSCTL_PATH instead, and if you'd like to update more plugins to follow, you're very welcome to do so 😃 👍

@aanderse
Copy link
Contributor Author

So please go for SYSCTL_PATH instead

ah, yeah i see now - thanks

You'll undoubtedly find more.

well... most of the others are different - they have the implicit assumption that PATH contains the executable in question, which is probably the best way to do it

procps.c is different than most other cases because it hardcodes a path (/sbin)
even your modules-load.c example isn't quite the same because it allows you to specify a path at "runtime" (ie. in the configuration file)

in fact... maybe we should just modify procps.c to assume sysctl exists on PATH like the rest... or are there implications to that i haven't considered? 🤔

@troglobit
Copy link
Owner

You'll undoubtedly find more.
well... most of the others are different - they have the implicit assumption that PATH contains the executable in question, which is probably the best way to do it

Hmm, yeah 🤔

procps.c is different than most other cases because it hardcodes a path (/sbin) even your modules-load.c example isn't quite the same because it allows you to specify a path at "runtime" (ie. in the configuration file)
in fact... maybe we should just modify procps.c to assume sysctl exists on PATH like the rest... or are there implications to that i haven't considered? 🤔

For most users that would probably be sufficient, yes. But I like that with your PR we now will get a way for users to override.

Also, as PID 1 the default PATH is usually _PATH_STDPATH which, unless user override, defaults to "/usr/bin:/bin:/usr/sbin:/sbin"

@aanderse
Copy link
Contributor Author

out of curiosity what is the difference between sysctl and mount in your mind then? in finit.c there is no way to satisfy requirements other than ensuring mount exists on PATH - no MOUNT_PATH (or i guess _PATH_MOUNT 😅) exists - while procps.c will ignore PATH and only observe compile time options.

maybe i am thinking too hard about a fairly small detail...

@troglobit
Copy link
Owner

out of curiosity what is the difference between sysctl and mount in your mind then? in finit.c there is no way to satisfy requirements other than ensuring mount exists on PATH - no MOUNT_PATH (or i guess _PATH_MOUNT 😅) exists - while procps.c will ignore PATH and only observe compile time options.

maybe i am thinking too hard about a fairly small detail...

There is no difference really, in my mind. Much of Finit grew organically, sometimes by external contributions, some hard-coded paths, others relied on $PATH. Personally I prefer to be more lax and allow finding sysctl and others in $PATH, but experience tells me people often have varying degrees of both knowledge and adherence to the FHS so we should prepare for both and make the common case simple.

This was a generic answer, maybe not what you were looking for?

I guess what I'd like is to have the following pattern in most/all places:

#ifndef FOO
#define FOO foo
#endif 


some_function()
{
    systemf(FOO "-some %s", args);
}

Which would make it possible to both rely on $PATH in the common case, and override in the specific case. Does that make sense?

@aanderse
Copy link
Contributor Author

great answer, what i was looking for

in that case... how about we merge this as it stands and in the future if i want i can reference this in a new PR?

thanks

@troglobit
Copy link
Owner

Yup 💯 just wanted to sync with you first.

@troglobit troglobit merged commit ecf65ce into troglobit:master Feb 21, 2025
2 checks passed
@aanderse aanderse deleted the path/sysctl branch February 21, 2025 03:48
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.

2 participants