Skip to content

Conversation

@metux
Copy link
Contributor

@metux metux commented May 25, 2021

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Comment on lines 117 to 81
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the shfmt settings we use doesn't like some of these formatting changes; https://ci-next.docker.com/public/blue/rest/organizations/jenkins/pipelines/moby/branches/PR-42420/runs/1/nodes/251/log/?start=0

You can check the format it wants with; shfmt -w -bn -ci -sr
Screenshot 2021-05-25 at 21 45 43

(The change of order of -p "$DOCKER_PIDFILE" it expects is a bit odd though; looks like it's getting confused by something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, since patch is just for aligning formatting to Debian's version (no idea which one was first) - to make diffing easier - I can move that one to my packager queue and drop it from this submission.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

If you're updating, could you also fix the typo in the first commit message? (s/DEAMON/DAEMON/)

@tianon
Copy link
Member

tianon commented May 25, 2021

I've got to be honest that most of this seems like changes just for changes sake.

For example, I don't see a compelling reason to adjust any of the whitespace in this file (especially since we're now using shfmt to keep all the scripts in the repository consistent), and renaming variables doesn't seem like it really adds much value? Some of your commit messages also reference "debian init script standards" but on my Debian Bullseye system I'm not seeing much consistency between any of the files I have in /etc/init.d, and as a Debian Developer I'm not familiar with any project-wide "standards" for init scripts. The closest thing to that I'm aware of is https://manpages.debian.org/buster/sysvinit-utils/init-d-script.5.en.html, which IMO would actually be a valuable conversion for us to make (but it is Debian-specific). None of the files on my system are using readlink for setting BASE/NAME.

The fact that https://salsa.debian.org/debian/sysvinit/-/blob/21ee620f9aac4d910a769cf3f84c783c19eac199/debian/init-d-script doesn't seem to muck with PID files any more than setting --pidfile makes we wonder if we're maybe doing more than we should be WRT pidfile-related options.

@metux metux force-pushed the submit/contrib-debian-initscript branch from 1343634 to 6c2da9d Compare June 16, 2021 13:58
@metux metux force-pushed the submit/contrib-debian-initscript branch 2 times, most recently from 3b712c1 to 528db9b Compare August 24, 2023 10:06
@metux
Copy link
Contributor Author

metux commented Aug 24, 2023

@tianon:

The retionale behind this queue: when packaging the upstream tree for Debian family, I've found lots of many differences between moby's and debian's versions. Lots of small things, but summing up to a huge diff.
Consolidating that would make tracking changes on both sides a lot easier.

One thing is actually making a big difference: the last patch is picking the config scriptlet name from the init script's name. If you wanna have multiple instances, you can just copy or hardlink the init script as-is and it will load a separate config. (yes, that's not the cleanest imaginable solutions, but works well enough and often used in the field).

@metux metux force-pushed the submit/contrib-debian-initscript branch 2 times, most recently from dca39d1 to 462721b Compare August 25, 2023 11:29
metux added 4 commits August 25, 2023 14:38
Make it a bit more aligned to debian init script standards.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make it a bit more aligned to debian init script standards.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make sure stale pidfiles are removed.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
automatically derive the service name from init script name.
this way, eg. one script can drive multiple instances.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
@metux metux force-pushed the submit/contrib-debian-initscript branch from 462721b to 1313552 Compare August 25, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants