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

add support for passing arguments to services #386

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

q66
Copy link
Sponsor Contributor

@q66 q66 commented Oct 2, 2024

The argument is a part of the full service name always. Therefore, each argument instantiation is a separate instance of the service, with separate loading.

The following cases are supported:

  1. passing the argument to any dinitctl command that takes an a service name
  2. dependencies with an argument
  3. using the argument in any context that does variable substitution
  4. using the argument in dependency arguments (performing a restricted version of variable substitution)

documentation is updated, load+integration tests are updated/added

while at it, i found that depends-ms.d was being parsed wrong (it was never reading the dir) so i fixed that too

@q66 q66 force-pushed the service-arg branch 2 times, most recently from 4a909f0 to c05717d Compare October 2, 2024 12:59
@mobin-2008 mobin-2008 added Bug/Bugfix Enhancement/New Feature Improving things or introduce new feature A-Importance: Normal C-dinitctl Things about dinitctl C-dinit-service Things about dinit-service C-dinitcheck Things about dinitcheck labels Oct 2, 2024
@q66
Copy link
Sponsor Contributor Author

q66 commented Oct 2, 2024

i'm a bit confused by the aarch64 failure after i force pushed?

@q66 q66 force-pushed the service-arg branch 2 times, most recently from dceaa89 to 1729dbc Compare October 2, 2024 15:39
@q66
Copy link
Sponsor Contributor Author

q66 commented Oct 2, 2024

actually that was probably just a readiness race for the dependency chain, i made all the dependencies scripted (that should address that)

@davmac314
Copy link
Owner

I had a quick look and it seems ok, I'll review properly on the weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Importance: Normal Bug/Bugfix C-dinit-service Things about dinit-service C-dinitcheck Things about dinitcheck C-dinitctl Things about dinitctl Enhancement/New Feature Improving things or introduce new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants