Respect enabled services and dropins, fix grep.

[ml-]

• grep only for enabled units and dropin files
• grep for exact match to avoid commented-out lines
• grep for one match, then continue
• check if service of dropin is actually enabled
• add dropin configuration of enabled units
• loop over systemd cat output to add dropin binaries as well
• ignore empty Exec* override keys

Previously the script tried to include *.pacsave files that were left after the latest upgrade for anyone who edited these files. Now since we only consider enabled services and match for only two file extensions this can't happen anymore.

Also it is no longer required to edit --full a service to add it, a drop-in will do the job.

[Andrei-Pozolotin]

@ml- Matthias:

1. very good points, thank you

2. did you test for all these permutations?

   * grep only for enabled units and dropin files
   * grep for exact match to avoid commented-out lines
   * grep for one match, then continue
   * check if service of dropin is actually enabled
   * add dropin configuration of enabled units
   * loop over systemd cat output to add dropin binaries as well
   * ignore empty Exec* override keys

3. also with changes due to the migration from aur into community? https://github.com/random-archer/mkinitcpio-systemd-tool/pull/30

4. please squash commits into 1 after you finish these tests

[dvzrv]

@ml- This is a indeed a good idea. Also, hi! :)

[ml-]

@Andrei-Pozolotin

2. I have tested tested all of the points for the initial commit, including reboot. I havent done the "reboot and hope for the best" test for the second commit from a few minutes ago. Will do asap.

3. It was because of the changes in that PR.

4. I will squash it to one commit when I've made sure everything works properly.

@dvzrv Hi. I have noticed that initrd-cryptsetup.service can no longer be added to initramfs since https://github.com/random-archer/mkinitcpio-systemd-tool/pull/30.

It was added prior this because besically everything in /etc/systemd/system was added. initrd-cryptsetup.path should pull it in implicitly, but the .service is not present now and can't be enabled explicitly.

For some (systemd-)reason decryption still works due to the default prompt showing up. (thank god) I'm not sure if it's the desired behavior, becaue I see this:

shell[326]: console/ssh info : init
shell[330]: console/ssh info : crypt jobs
shell[333]: console/ssh info : custom agent try #1
shell[342]: console/ssh info : query start
shell[347]: console/ssh error: query failure [-sh: /usr/bin/systemd-ask-password: not found]
shell[349]: console/ssh info : system agent

Can you see this as well?

Adding initrd-cryptsetup.service to Requires= of the .path unit explicitly seems to solve the problem.

tl;dr: tiny side effect caused by the PR yesterday, want me to fix it here?

[Andrei-Pozolotin]

want me to fix it here?

yes, please

[ml-]

Ok, this builds and includes all the files (i think). Will continue after a short break and see if everything is alive after the reboot.

Edit: and squash ofc

[dvzrv]

@dvzrv Hi. I have noticed that initrd-cryptsetup.service can no longer be added to initramfs since #30.

It was added prior this because besically everything in /etc/systemd/system was added. initrd-cryptsetup.path should pull it in implicitly, but the .service is not present now and can't be enabled explicitly.

For some (systemd-)reason decryption still works due to the default prompt showing up. (thank god) I'm not sure if it's the desired behavior, becaue I see this:

shell[326]: console/ssh info : init
shell[330]: console/ssh info : crypt jobs
shell[333]: console/ssh info : custom agent try #1
shell[342]: console/ssh info : query start
shell[347]: console/ssh error: query failure [-sh: /usr/bin/systemd-ask-password: not found]
shell[349]: console/ssh info : system agent

Can you see this as well?

Adding initrd-cryptsetup.service to Requires= of the .path unit explicitly seems to solve the problem.

Ouf, didn't notice that! Then again, would the specific initrd-cryptsetup.service still be required at all then?

[ml-]

Ouf, didn't notice that! Then again, would the specific initrd-cryptsetup.service still be required at all then?

I had the very same question. @Andrei-Pozolotin can probably give the best answer

Edit: but even if it isn't required, some changes need to be made to (re-)start the secret agent from the shell

[ml-]

Ok, did a reboot with initrd-cryptsetup.path enabled as well. Everything looks good (Works for me™). I squashed the change.

Please also consider that this might actually break things for users who have used this package without explicitly enabling services that are located directly in /etc/systemd/system.

[Andrei-Pozolotin]

1. will deal with *.path issue later
2. @ml- Matthias: thank you for your work
3. @dvzrv David: please release community

[Andrei-Pozolotin]

@ml- Matthias, @dvzrv David:

while the dust settles, what do you guys think:

original intent of InitrdService was to auto-magically drive enable/disable of the service from the unit file itself, during the mkinitcpio build invocation

[X-SystemdTool]
# enable service
InitrdService=enable

then, should we:

• restore/fix this behavior
• remove InitrdService completely, require user action as it is now
• or ...?

[ml-]

then, should we:

• restore/fix this behavior
• remove InitrdService completely, require user action as it is now
• or ...?

The way I see it: systemd should decide what's enabled, and systemd-tool should do the same. Not only is this a well known procedure for the user, it will also be easier for maintainers/developers to handle systemd units inside the scripts.

Besides this, I have to admit that I was confused by the InitrdService directive and didn't quiet get the purpose, especially after the move to community where services had to be explicitly enabled.

• restore behavior: no
• remove InitrdService: yes!

[dvzrv]

@Andrei-Pozolotin Yes, I also think, that relying on systemd's functionality of enabled services here is the way to go (and less confusing to the user).

Less code, less errors! :D

[Andrei-Pozolotin]

ok, remove it is, thank you guys.