Bug 252054 - mergemaster(8) regression after transitioning from svn to git
Summary: mergemaster(8) regression after transitioning from svn to git
Status: Closed Overcome By Events
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-22 18:55 UTC by Marek Zarychta
Modified: 2021-01-04 20:16 UTC (History)
5 users (show)

See Also:


Attachments
mergemaster run with -FU still prompting to update the same file (18.14 KB, text/plain)
2020-12-22 18:55 UTC, Marek Zarychta
no flags Details
Ugly hack for mergemaster(8) to workaround Id removal (1.08 KB, patch)
2020-12-22 22:04 UTC, Marek Zarychta
no flags Details | Diff
Still uggly but better suited workaround (757 bytes, patch)
2020-12-22 22:48 UTC, Marek Zarychta
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Zarychta 2020-12-22 18:55:40 UTC
Created attachment 220823 [details]
mergemaster run with -FU  still prompting to update the same file

mergemaster(8) has -F option which allows you to merge the files if the file header has updated or changed. Git doesn't do ident tag expansion so in file header you have now only $FreeBSD$. When both -FU options are used, mergemaster prompts only to merge when the header is different. When the header is in svn style, after updating the ident tag mergemaster doesn't ask again for updating the same file in second and subsequent runs. The transition to git has broken this feature and mergemaster prompts for the merge or update also in second and subsequent runs. Please compare the attached script.
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2020-12-22 19:11:20 UTC
I looked at this for a bit, and this is kind of gross behavior on mergemaster's part and not specific to -F:

https://cgit.freebsd.org/src/tree/usr.sbin/mergemaster/mergemaster.sh#n1122 says:

  case "${STRICT}" in
  '' | [Nn][Oo])
    # Compare $Id's first so if the file hasn't been modified
    # local changes will be ignored.
    # If the files have the same $Id, delete the one in temproot so the
    # user will have less to wade through if files are left to merge by hand.
    #
    ID1=`grep "[$]${ID_TAG}:" ${DESTDIR}${COMPFILE#.} 2>/dev/null`
    ID2=`grep "[$]${ID_TAG}:" ${COMPFILE} 2>/dev/null` || ID2=none

    case "${ID2}" in
    "${ID1}")
      echo " *** Temp ${COMPFILE} and installed have the same Id, deleting"
      rm "${COMPFILE}"
      ;;
    esac
    ;;
  esac

So if the ident tag was expanded and it matched, mergemaster would assume no changes to the file and discard the new version. I suspect this is a WONTFIX situation, unfortunately.
Comment 2 Kyle Evans freebsd_committer freebsd_triage 2020-12-22 19:28:17 UTC
(In reply to Kyle Evans from comment #1)

The behavior is less gross than I stated in that comment, but I did weird stuff here where strict would've been a better default.
Comment 3 Marek Zarychta 2020-12-22 19:34:02 UTC
The behaviour of mergemaster is fine. The drawback is that you need to reserve a lot more time to update config files with mergemaser and the option -F became useless.
Comment 4 Marek Zarychta 2020-12-22 22:04:22 UTC
Created attachment 220827 [details]
Ugly hack for mergemaster(8) to workaround Id removal

With this patch running "mergemaster -UFi" is even riskier than before, but some might like it. Perhaps the right solution is not to workaround this but to add a new option like -T to replace -F to mergemaster(8).
Comment 5 Marek Zarychta 2020-12-22 22:48:13 UTC
Created attachment 220828 [details]
Still uggly but better suited workaround

Better suited patch, still a workaround.
Comment 6 Marek Zarychta 2020-12-22 23:23:26 UTC
(In reply to Marek Zarychta from comment #5)
I blindly assumed that timestamps in TEMPROOT dir will be preserved but it's completely wrong assumption making this workaround useless. The files are not just copied or installed but make(1) is involved in TEMPROOT setup.
Comment 7 Marek Zarychta 2020-12-22 23:42:14 UTC
People these times do not update their OSes from the sources. It's probably high time to deprecated this old method from the previous century and transition to freebsd-update(8).
Comment 8 Mark Millard 2020-12-23 00:32:31 UTC
(In reply to Marek Zarychta from comment #7)

freebsd-update use is not viable for some platforms/contexts.

There are also folks that work on updating FreeBSD beyond what
freebsd-update has available at the time (updates will become
available later).

Getting build-from-source to have zero use is problematical.
Comment 9 Marek Zarychta 2020-12-23 08:32:07 UTC
The project can utilise gitatributes(5) to support ident as $Id:$ in Git. The specific files or directories could have it enabled and probably both: mergemaster(8) and etcupdate(8) after small rework can fully utilise it.

The comment #7 suggests that updating from sources is still viable so I reopen it.
Comment 10 Thierry Thomas freebsd_committer freebsd_triage 2020-12-23 10:43:28 UTC
(In reply to Marek Zarychta from comment #7)

freebsd-update does not support -STABLE.
Comment 11 Marek Zarychta 2020-12-23 10:49:17 UTC
(In reply to Thierry Thomas from comment #10)
Yes, I closed the bug with this comment. But comment #8 says that not everyone is willing to use only freebsd-update to update the system, so the bug got reopened.
Comment 12 Mark Linimon freebsd_committer freebsd_triage 2020-12-23 12:21:59 UTC
(In reply to Marek Zarychta from comment #11)

freebsd-update is not available for many (all?) of our tier-2 architectures.
Comment 13 Kyle Evans freebsd_committer freebsd_triage 2020-12-23 14:04:40 UTC
There's a very real chance that mergemaster could just get deprecated here instead. As I mentioned on IRC, you should give etcupdate a shot.
Comment 14 Mark Millard 2020-12-23 14:35:04 UTC
(In reply to Kyle Evans from comment #13)

I'm not aware of documentation of the equivalents for etcupdate use
to the standard documentation's use of specific mergemaster commands
for variations of the source based update sequence, such as in
/usr/src/Makefile :

. . .
#  5.  `reboot'        (in single user mode: boot -s from the loader prompt).
#  6.  `mergemaster -p'
#  7.  `make installworld'
#  8.  `mergemaster'            (you may wish to use -i, along with -U or -F).
. . .

or on the COMMON ITEMS in UPDATING.
Comment 15 Marek Zarychta 2020-12-23 14:59:58 UTC
(In reply to Kyle Evans from comment #13)
If mergemaster(8) gets deprecated then people would say goodbye to it and will struggle with etcupdate(8). Mergemaster does the merge side by side, etcupdate adds "<<<<<<<<<<<<<<<" ">>>>>>>>>>>>>>" to merged files and you have to edit them manually also taking care of these ">>>>>>>>>>>>>>" marks.

Is it that big effort to add ident among other gitattributes(5) to have $Is$ tags to this part of sources? They are only needed for the files mergemaster or etcupdate has to cope with. When it's done both tools can be patched manually by FreeBSD enduser if he would care.

Even if mergemaster gets deprecated and removed from the base such ident tags in config files will be helpful. How without them would non-committers who update their repositories only occasionally would figure out that the file has changed?
Comment 16 Marek Zarychta 2020-12-23 17:51:06 UTC
There are no plans to support ident $Id$, as implemented in git - it is official information. So kevans@ is right that it is a WONTFIX situation. mergemaster is still able to do the job and doesn't have to be deprecated due to this breakage.
 
I have submitted this PR but I think it is time to close it definitely, not look back, enjoy and celebrate the transition to the git repo.