Bug#1023317: Bug#1023318: / Bug#1023317: {screen,zsh}: re-adds /usr/bin/{screen,zsh} to /etc/shells on upgrade

2022-11-03 Thread Axel Beckert
Hi Helmut,

Helmut Grohne wrote:
> On Thu, Nov 03, 2022 at 12:26:18AM +0100, Axel Beckert wrote:
> > Some more hints about how this all is meant to work and why it
> > actually works would have been appreciated, though:
> 
> Thank you for the quick reply and thank you for bringing up the lack of
> documentation.

You're welcome. And thanks for not being annoyed of my reply. :-)

> > Helmut Grohne wrote:
> > It took me quite a while to where this
> > /usr/share/debianutils/shells.d/ comes from and where it is this
> > documented,
> 
> I'm sorry for using your time in such a bad way. Consider asking for
> feedback earlier next time.

Well, you know, I first try to figure out things myself before asking
others too quickly. And I usually dig further if I have the feeling,
I'm very close to the answer. In the end you got my way through the
problem as I tend to protocol what I've done.

> > /usr/share/doc/debianutils/README.shells.gz clearly states that I
> > should use add-shell and remove-shell:
> 
> In all my work, I happen to not have run into this, but I really should.
> Thank you for pointing it out. Failing to update it is an obvious
> omission in retrospect and this is the place that should have told you
> all that you wanted to know.

Indeed.

> I have thus filed #1023380 to fix that.

Thanks a lot! Looks good to me and clearly improves the situation a lot.

> Please let me know if you miss anything else.

Maybe the man pages add-shell(8) and remove-shell(8) should mention
update-shells(8) under "SEE ALSO" or mention that there's also a
trigger-based invocation of them (with pointers then). That would have
helped me as well. (Cc'ing #1023380 for these suggestions.)

P.S.: Will work on the two issues you've reported soon-ish. Thanks
again for making us aware of them.

Regards, Axel
-- 
 ,''`.  |  Axel Beckert , https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-|  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE



Bug#1023317: Bug#1023318 / Bug#1023317: {screen,zsh}: re-adds /usr/bin/{screen,zsh} to /etc/shells on upgrade

2022-11-03 Thread Helmut Grohne
Hi Axel,

On Thu, Nov 03, 2022 at 12:26:18AM +0100, Axel Beckert wrote:
> Some more hints about how this all is meant to work and why it
> actually works would have been appreciated, though:

Thank you for the quick reply and thank you for bringing up the lack of
documentation.

> Helmut Grohne wrote:
> It took me quite a while to where this
> /usr/share/debianutils/shells.d/ comes from and where it is this
> documented,

I'm sorry for using your time in such a bad way. Consider asking for
feedback earlier next time.

> /usr/share/doc/debianutils/README.shells.gz clearly states that I
> should use add-shell and remove-shell:

In all my work, I happen to not have run into this, but I really should.
Thank you for pointing it out. Failing to update it is an obvious
omission in retrospect and this is the place that should have told you
all that you wanted to know. I have thus filed #1023380 to fix that.

Please let me know if you miss anything else.

Helmut



Bug#1023317: Bug#1023318 / Bug#1023317: {screen,zsh}: re-adds /usr/bin/{screen,zsh} to /etc/shells on upgrade

2022-11-02 Thread Axel Beckert
Hi Helmut,

thanks for these bug reports and making us aware of this type of
issue.

Some more hints about how this all is meant to work and why it
actually works would have been appreciated, though:

Helmut Grohne wrote:
> When upgrading or reinstalling screen, it adds /usr/bin/screen to
> /etc/shells even if one locally removed it there. Such behaviour
> violates Debian policy section 10.7.3. I propose managing the entry
> declaratively using dpkg triggers and am attaching a patch for your
> convenience.
> + install -D -m644 debian/shells 
> $(ROOT)/usr/share/debianutils/shells.d/screen
[…]
> -  add-shell /usr/bin/screen || true
[…]
> -if [ "$1" = disappear ]; then
> -  remove-shell /usr/bin/screen || true
> -fi

It took me quite a while to where this
/usr/share/debianutils/shells.d/ comes from and where it is this
documented,

/usr/share/doc/debianutils/README.shells.gz clearly states that I
should use add-shell and remove-shell:

| So, if a package contains something that the maintainer thinks ought
| to be a valid login shell, it's postinst should, (on initial install
| only, to allow a sysadmin to take it out again), run:
|
| /usr/sbin/add-shell /path/to/shell
|
| In the postrm, probably on remove, the package should call
| 
| /usr/sbin/remove-shell /path/to/shell
| 
| As the various shells start to use it, the default shells list will
| shrink.

No mention of that directory. The man pages add-shell(8) and
remove-shell(8) don't mention this directory either.

Also can't find it mentioned in
/usr/share/doc/debianutils/changelog.Debian.gz.

I also found no mentioning of triggers inside the functional part of
the patches. So I assume you meant to refer to
/var/lib/dpkg/info/debianutils.triggers (which also seems not to be
mentioned in /usr/share/doc/debianutils/changelog.Debian.gz).

Finally when looking /var/lib/dpkg/info/debianutils.postinst, I found
a pointer to update-shells whose man-page explains that whole
mechanism and also why it actually track changes made by the
administrator. An early pointer to that or #990440 would have been
nice…

Regards, Axel
-- 
 ,''`.  |  Axel Beckert , https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-|  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE