Page MenuHomeFreeBSD
Differential D24240

Improve and extend sed_checked.sh
Needs ReviewPublic
Actions

Authored by mandree on Mar 31 2020, 3:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 31, 4:31 AM
Unknown Object (File)
Feb 8 2024, 7:08 AM
Unknown Object (File)
Jan 26 2024, 6:57 PM
Unknown Object (File)
Dec 28 2023, 11:11 AM
Unknown Object (File)
Dec 22 2023, 11:17 PM
Unknown Object (File)
Dec 14 2023, 6:51 PM
Unknown Object (File)
Nov 15 2023, 9:26 AM
Unknown Object (File)
Nov 14 2023, 7:58 AM
View All 44 Files
Subscribers
emaste
imp
manu
mat

Details

Reviewers
mandree
Group Reviewers
portmgr
O5: Ports Framework
Commits
rP552563: Tools/scripts/sed_checked.sh: make more useful
Summary

The current sed_checked.sh validates REINPLACE_CMD expressions in port Makefiles.
It has a few shortcomings that this diff strives to address:

  • the test is blind to whether a REINPLACE_CMD line is completely useless, or partially ineffective - report that because for some machine edits, a broader glob has been common practice
  • REINPLACE_CMD may suppress creation of backup files (-i ''), which invalidates this check, so warn about that
  • report found data immediately, and again through REWARNFILE.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mandree created this revision.Mar 31 2020, 3:53 PM
mandree created this object with edit policy "committers (Project)".
Herald added a subscriber: mat. · View Herald TranscriptMar 31 2020, 3:53 PM
mandree updated this revision to Diff 71702.May 12 2020, 8:32 PM

same code, rediffed with "full" context

mandree added a comment.Jun 4 2020, 9:23 AM

ping

mandree added a subscriber: manu.Jun 16 2020, 2:02 PM

@portmgr @manu @mat @swills

Guys, this has been sitting for five weeks since the full context was provided, and almost three months since initial review request.
Can we get this moving? Can I get approval for commit? I've been using this for 10+ weeks.

mandree added a comment.Jul 16 2020, 3:03 PM

No more complaints received in a month, can I commit?

mandree accepted this revision.Sep 23 2020, 4:56 PM

ping

This revision is now accepted and ready to land.Sep 23 2020, 4:56 PM
mandree removed a reviewer: swills.Sep 23 2020, 4:56 PM
mat added a comment.Sep 30 2020, 8:40 AM
In D24240#568481, @mandree wrote:

No more complaints received in a month, can I commit?

We do not have more complains, but we still do not agree with the change, so the answer is still no.

mandree added a comment.Sep 30 2020, 2:12 PM

OK, then this will escalate to core@ for abuse because there has been no feedback as to why it's unacceptable in spite of solving practical problems.

imp added a subscriber: imp.Sep 30 2020, 3:13 PM
In D24240#592496, @mat wrote:
In D24240#568481, @mandree wrote:

No more complaints received in a month, can I commit?

We do not have more complains, but we still do not agree with the change, so the answer is still no.

Can you articulate a reason? A blanket "no" without an actionable explanation is frowned upon. The code looks fine. The concept seems on its face OK. What's the holdup?

emaste added a subscriber: emaste.Sep 30 2020, 7:51 PM
Closed by commit rP552563: Tools/scripts/sed_checked.sh: make more useful (authored by mandree). · Explain WhyOct 17 2020, 12:37 PM
This revision was automatically updated to reflect the committed changes.
mandree added a commit: rP552563: Tools/scripts/sed_checked.sh: make more useful.
Owners added a reviewer: O5: Ports Framework.Oct 17 2020, 12:37 PM
Herald added 1 blocking reviewer(s): portmgr. · View Herald TranscriptOct 17 2020, 12:37 PM
mandree reopened this revision.Jan 4 2021, 6:15 PM

since swills@ has reverted this as of r552736, reopen.

mandree mentioned this in D27954: BUGFIX: Only call sed_checked.sh if REINPLACE_ARGS hasn't been altered.Jan 4 2021, 6:17 PM
mandree added a comment.Feb 14 2021, 10:53 AM

In the light of findings mentioned in D27954 we might want to add removal of .bak files, but other than that my proposal already looks in-depth at the REINPLACE_ARGS, to complain if something uses a nonstandard -i argument (backup suffix).

Revision Contents
Changeset List

PathSizePackages
head/
Mk/
Scripts/
91 linesO5: Ports Framework

Diff 78346

View Options

head/Mk/Scripts/sed_checked.sh

Loading...