Bug 242212 - usr.sbin/mergemaster/mergemaster.sh: There is no logic to handle symbolic links
Summary: usr.sbin/mergemaster/mergemaster.sh: There is no logic to handle symbolic links
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Warner Losh
URL:
Keywords: feature, needs-qa
Depends on:
Blocks: 249544
  Show dependency treegraph
 
Reported: 2019-11-24 23:29 UTC by Yasuhiro Kimura
Modified: 2021-07-08 05:14 UTC (History)
4 users (show)

See Also:
koobs: mfc-stable12?
koobs: mfc-stable11?


Attachments
Patch file (3.67 KB, patch)
2020-01-16 07:55 UTC, Yasuhiro Kimura
no flags Details | Diff
Updated patch file (4.18 KB, patch)
2020-01-22 06:46 UTC, Yasuhiro Kimura
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-24 23:29:39 UTC
With base r354922 /usr/src/etc/Makefile is changed as following.

yasu@rolling-vm-freebsd1[2109]% LANG=C svn diff -c 354922 /usr/src/etc/Makefile
Index: /usr/src/etc/Makefile
===================================================================
--- /usr/src/etc/Makefile       (revision 354921)
+++ /usr/src/etc/Makefile       (revision 354922)
@@ -57,6 +57,8 @@
        ${_+_}cd ${.CURDIR}/mtree; ${MAKE} install
        ${_+_}cd ${SRCTOP}/share/termcap; ${MAKE} etc-termcap
        ${_+_}cd ${SRCTOP}/usr.sbin/rmt; ${MAKE} etc-rmt
+       ${INSTALL_SYMLINK} ../var/run/os-release \
+               ${DESTDIR}/etc/os-release
 .if ${MK_UNBOUND} != "no"
        if [ ! -e ${DESTDIR}/etc/unbound ]; then \
                ${INSTALL_SYMLINK} -T "package=unbound" \
yasu@rolling-vm-freebsd1[2110]%

This means /etc/os-release symbolic link is created when 'make distribution' is executed. But this target isn't executed at normal upgrade steps. So as a result /etc/os-release symbolic link isn't created when you upgrade an existing 13-CURRENT host.
Comment 1 Warner Losh freebsd_committer freebsd_triage 2019-11-24 23:32:57 UTC
make distribution is run as part of mergemaster / etcupdate.

Have you done that step? When I did, my system grew this symlink.
Comment 2 Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-24 23:46:52 UTC
(In reply to Warner Losh from comment #1)

Yes. I ran 'mergemaster -Fi' after 'make installworld' when upgrading from base r354592 to base r355028. But /etc/os-release isn't created.
Comment 3 Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-25 00:11:29 UTC
(In reply to Yasuhiro KIMURA from comment #2)

I upgraded my 13-CURRENT host from base r355028 to base r355059 and 'mergemaster -Fi' didn't created /etc/os-release again.
Comment 4 Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-25 01:35:47 UTC
(In reply to Yasuhiro KIMURA from comment #3)

According to the report in freebsd-current ML this problem seems to be bug of mergemaster.

https://lists.freebsd.org/pipermail/freebsd-current/2019-November/074852.html
Comment 5 Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-25 08:35:26 UTC
(In reply to Yasuhiro KIMURA from comment #4)

While mergemaster is running there are following symbolic links under /var/tmp/temproot.

root@rolling-vm-freebsd1[2009]# LANG=C find -s /var/tmp/temproot -type l -ls
 10750        1 lrwxr-xr-x    1 root                             wheel                                  12 Nov 25 17:20 /var/tmp/temproot/etc/aliases -> mail/aliases
 11922        1 lrwxr-xr-x    1 root                             wheel                                  21 Nov 25 17:20 /var/tmp/temproot/etc/os-release -> ../var/run/os-release
 11921        1 lrwxr-xr-x    1 root                             wheel                                  15 Nov 25 17:20 /var/tmp/temproot/etc/rmt -> ../usr/sbin/rmt
 11845        1 lrwxr-xr-x    1 root                             wheel                                  23 Nov 25 17:20 /var/tmp/temproot/etc/termcap -> /usr/share/misc/termcap
 10747        1 lrwxr-xr-x    1 root                             wheel                                  14 Nov 25 17:20 /var/tmp/temproot/etc/unbound -> ../var/unbound
root@rolling-vm-freebsd1[2009]#

So I removed following symbolic links and then ran mergemaster.

/etc/aliases
/etc/os-release
/etc/rmt
/etc/termcap
/etc/unbound

But none of them were installed. This means mergemaster doesn't install symbolic links even if they don't exist.
Comment 6 Yasuhiro Kimura freebsd_committer freebsd_triage 2019-11-25 16:23:54 UTC
I made upgrade test of 13-CURRENT with following conditions.

* Clean install 13-CURRENT base r354699 amd64 by using ISO image of 2019/11/14 head snapshot (latest snapshot that don't install /etc/os-release).
* Check out source tree of head base r355087.
* Do exact upgrade steps written in /usr/src/Makefile.

And as a result there isn't /etc/os-release after upgrade.
Comment 7 Yasuhiro Kimura freebsd_committer freebsd_triage 2020-01-16 07:55:21 UTC
Created attachment 210786 [details]
Patch file

I investigated mergemaster and found there is no logic in it to install symbolic link . Attached patch fixes it. To be honest I don't fully upderstand all functions of mergemaster and therefore this patch requires review of someone else who is familiar with it. But I hope my patch is of sufficient quality as a starting point.
Comment 8 Yasuhiro Kimura freebsd_committer freebsd_triage 2020-01-22 06:46:56 UTC
Created attachment 210946 [details]
Updated patch file

* Change and/or fix some messages.
* Don't delete ${TEMPROOT} without confirmation when symbolic links are left in it.
* Delete temp symbolic link when it is successfully installed.

So please review/test attached patch instead of original one.
Comment 9 Warner Losh freebsd_committer freebsd_triage 2020-11-18 19:23:04 UTC
I've committed r367810 which should fix this. I'll try to remember to MFC in 2 weeks.
Comment 10 commit-hook freebsd_committer freebsd_triage 2020-11-18 19:23:22 UTC
A commit references this bug:

Author: imp
Date: Wed Nov 18 19:22:24 UTC 2020
New revision: 367810
URL: https://svnweb.freebsd.org/changeset/base/367810

Log:
  mergemaster: handle symbolic links during update.

  /etc/os-release is now a symbolic link to a generated file. Make
  mergemaster cope with symbolic links generically. I'm no longer
  a big mergemaster user, so this has only been lightly tested
  by me, though Kimura-san has ran it through its paces.

  Submitted by: Yasushiro KIMURA-san
  PR: 242212
  MFC After: 2 weeks

Changes:
  head/usr.sbin/mergemaster/mergemaster.sh
Comment 11 Mark Linimon freebsd_committer freebsd_triage 2021-05-11 15:10:31 UTC
^Triage: assign as possible MFC reminder.
Comment 12 commit-hook freebsd_committer freebsd_triage 2021-07-08 05:10:44 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=427654eb6a1435acb15c272731fd9518b11961d7

commit 427654eb6a1435acb15c272731fd9518b11961d7
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2020-11-18 19:22:24 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-08 05:10:05 +0000

    mergemaster: handle symbolic links during update.

    /etc/os-release is now a symbolic link to a generated file. Make
    mergemaster cope with symbolic links generically. I'm no longer
    a big mergemaster user, so this has only been lightly tested
    by me, though Kimura-san has ran it through its paces.

    Submitted by: Yasushiro KIMURA-san
    PR: 242212
    MFC After: 2 weeks

    (cherry picked from commit 30a56f9ef72705f2f3646ce4330cbc20675a465e)

 usr.sbin/mergemaster/mergemaster.sh | 118 +++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)
Comment 13 commit-hook freebsd_committer freebsd_triage 2021-07-08 05:13:45 UTC
A commit in branch stable/11 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=27e18c06f08b413cd6d90922971194ae65921805

commit 27e18c06f08b413cd6d90922971194ae65921805
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2020-11-18 19:22:24 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-08 05:13:18 +0000

    mergemaster: handle symbolic links during update.

    /etc/os-release is now a symbolic link to a generated file. Make
    mergemaster cope with symbolic links generically. I'm no longer
    a big mergemaster user, so this has only been lightly tested
    by me, though Kimura-san has ran it through its paces.

    Submitted by: Yasushiro KIMURA-san
    PR: 242212
    MFC After: 2 weeks

    (cherry picked from commit 30a56f9ef72705f2f3646ce4330cbc20675a465e)

 usr.sbin/mergemaster/mergemaster.sh | 118 +++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)