Page MenuHomeFreeBSD

UPDATING: Change update procedure to use etcupdate(8) over mergemaster(8)
ClosedPublic

Authored by driesm on Jan 9 2021, 1:57 PM.

Details

Summary

This changes the procedure in UPDATING to use etcupdate over megemaster.
This follows the recommendations in the handbook commited from D27848.

Commit message:

UPDATING: Change update procedure to use etcupdate(8) over mergemaster(8)

This commit aligns the steps in UPDATING with the steps from the handbook which already prefers etcupdate(8). While here also remove a dubious comment.

 PR: 252417
 Reviewed by: 0mp, ceri, imp
 Approved by: philip (mentor)
 Differential Revision: https://reviews.freebsd.org/D28062
`

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

driesm requested review of this revision.Jan 9 2021, 1:57 PM
driesm created this revision.

Could you please add PR: 252417 to the commit message?

UPDATING
2552

It never hurts to do it all the time.

This advice can actually break a system if the new etcupdate relies on behavior/code from the newly built world.

Please remove this comment or add similar advice to the original advice, noting the potential pitfall of using etcupdate(8), which can be incompatible with the old installed world.

2552

This was also the case for mergemaster too. Oof... that was bad advice.

Thanks for the review! I don't have a bit but I have updated the description with the PR number.

UPDATING
2552

So the comment that was there before is also applicable for worlds after 20130425 and 20130430? As the message only hints for the pitfall if we are updating from before these revisions (the way I interpret it, possibly incorrect though). Or is it a possible pitfall in general? If so I would just drop the specific revisions mentioned there as its confusing.

My eyeballs tell me these changes are good, but I've not 'play tested' them in anger to be sure.

UPDATING
2557

This change is independent, but imho, not important enough to do as a separate commit.

This revision is now accepted and ready to land.Mar 17 2021, 8:21 PM

Hi @debdrup, as you landed my review for updating the handbook to reference etcupdate, do you mind taking this one too? Thanks

grahamperrin_gmail.com added inline comments.
UPDATING
2464

Move up, to be inline.

2465

Remove white space.

2502

Move up, to be inline.

2503

Remove white space.

2507

I know that deleting old libraries can cause breakage (if performed carelessly).

Is there also risk of breakage for delete-oldwithout attention to libraries?

I routinely:

make -DBATCH_DELETE_OLD_FILES delete-old

– and never encountered breakage. Touch wood.

2532

Take this opportunity to renumber 3, 4, 5, 6, 8 and 9, and the referring points?

(There is no footnote 2 or 7.)

Then – order – begin the points, and their footnotes, at 1 instead of 7.

2578

Add white space between this line and the subheading that follows.

2589

1998–2021

UPDATING
2589

I've not been the majority contributor since 2009.

UPDATING
2432

if there is no EFI system partition

driesm marked an inline comment as done.

No real change intended rebased to master.

This revision now requires review to proceed.Apr 22 2021, 6:46 PM
driesm marked 9 inline comments as done.EditedApr 22 2021, 6:51 PM

In general your comments are valid and will open different reviews for some of them. But I'd like to keep this one just simple to change the mergemaster commands to etcupdate.

UPDATING
2464

It is intended like this so that it is not associated with make installkernel, it is seen as a seperate step between installkernel and reboot to single user mode.

2507

I'm trying to focus this review on mergemsater => Etcupdate. I've already folded in one unrelated change. I hope you understand.

2532

I'm trying to focus this review on mergemsater => Etcupdate. I've already folded in one unrelated change. I hope you understand.

In D28062#671670, @driesm.michiels_gmail.com wrote:

… keep this one just simple to change the mergemaster commands to etcupdate.

Understood and agreed; https://reviews.freebsd.org/D29934 noted with thanks.

Hi all, could this regain some traction?
The conflicting information between the handbook and the notes in UPDATING is confusing a few people on the forums etc.

Change looks good, I think, but is there any reason not to renumber those "footnotes" at the same time so that they are vaguely in order and 2 and 7 aren't missing for no good reason?

In D28062#690276, @ceri wrote:

Change looks good, I think, but is there any reason not to renumber those "footnotes" at the same time so that they are vaguely in order and 2 and 7 aren't missing for no good reason?

I try to keep the reviews small so they are easier to review. I made a separate one for that.
And yes, it is way better after reordering :-) https://reviews.freebsd.org/D29934

This revision is now accepted and ready to land.Jun 11 2021, 4:06 PM

I don't really have time for this right now, so someone else can go ahead and commit it once it's been accepted.

I don't really have time for this right now, so someone else can go ahead and commit it once it's been accepted.

@ngie - you’re holding the PR; will you commit?

Hi ceri, Warner accept a previous version, is that enough? The patch hasn't changed since then, I just rebased, content remains the same.

I accept this version two: I just have a quick comment about updating EFI boot that needn't be addressed in this.

UPDATING
2432

No. ****N E V E R **** use gpart bootcode on a UEFI system. It's always wrong now.

Anyone taking care of the commit? The handbook and UPDATING still give different step by step ATM ...

driesm added reviewers: philip, 0mp.

As I now have some (limited) power of my own I'd like to land this with the necessary approvals.

Given this is a change to the src repo, I'll need an approval from someone who contains a src commit bit, if I read this right in the committers guide :-).
Philip is this something you can provide? Technically, imp already accepted as well. Do I put both of you in the Approved by field?

Thanks for the feedback!

driesm retitled this revision from Change procedure in UPDATING to use etcupdate(8) over mergemaster(8) to UPDATING: Change update procedure to use etcupdate(8) over mergemaster(8).Tue, Nov 9, 2:55 PM
driesm edited the summary of this revision. (Show Details)

I'll reiterate that the green/red changes look good.
I want more, of course, but better is better, right?
tl;dr: go for it with my blessing.

Approved by: philip (mentor)