Page MenuHomeFreeBSD
Differential D38754

sysutils/debootstrap: Fix several issues
ClosedPublic
Actions

Authored by zirias on Feb 24 2023, 8:36 AM.
Tags
None
Referenced Files
F100627642: D38754.diff
Thu, Oct 17, 2:09 PM
F100597342: D38754.diff
Thu, Oct 17, 5:34 AM
Unknown Object (File)
Wed, Oct 16, 11:22 AM
Unknown Object (File)
Tue, Oct 15, 2:28 PM
Unknown Object (File)
Mon, Oct 14, 10:49 PM
Unknown Object (File)
Mon, Oct 14, 3:54 AM
Unknown Object (File)
Sun, Oct 13, 4:45 PM
Unknown Object (File)
Sat, Oct 12, 7:47 PM
View All Files
Subscribers
mat
rfyu28uyeg_snkmail.com

Details

Reviewers
nc
tcberner
arrowd
Commits
R11:4e0f740ddca6: sysutils/debootstrap: Fix several issues
Summary
  • PORTVERSION may not contain '-', use 'n' instead to include the nmu-number from debian. (Pointy-hat: zirias)
  • Don't use substitutions on Makefile, pass variables instead
  • Eliminate "static" substitutions, use a patch-file instead
  • Fix hardcoded /usr/local in debootstrap script

PR: 269584

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 49986
Build 46878: arc lint + arc unit

Event Timeline

zirias created this revision.Feb 24 2023, 8:36 AM
Herald added a subscriber: mat. · View Herald TranscriptFeb 24 2023, 8:36 AM
zirias requested review of this revision.Feb 24 2023, 8:36 AM
Harbormaster completed remote builds in B49985: Diff 117856.Feb 24 2023, 8:36 AM
arrowd accepted this revision.Feb 24 2023, 8:43 AM
This revision is now accepted and ready to land.Feb 24 2023, 8:43 AM
zirias updated this revision to Diff 117857.Feb 24 2023, 8:48 AM

Also remove substitutions that are now no-ops.

This revision now requires review to proceed.Feb 24 2023, 8:48 AM
Harbormaster completed remote builds in B49986: Diff 117857.Feb 24 2023, 8:48 AM
zirias updated this revision to Diff 117868.Feb 24 2023, 8:56 AM

And also make portclippy happy, as it's almost for free here

Harbormaster completed remote builds in B49987: Diff 117868.Feb 24 2023, 8:56 AM
zirias updated this revision to Diff 117869.Feb 24 2023, 9:26 AM

Rebase after committing the PORTVERSION fix separately

Harbormaster completed remote builds in B49988: Diff 117869.Feb 24 2023, 9:26 AM
arrowd accepted this revision.Feb 24 2023, 9:43 AM
This revision is now accepted and ready to land.Feb 24 2023, 9:43 AM
zirias updated this revision to Diff 117882.Feb 24 2023, 4:31 PM

Fix more hardcoded /usr/local after comment in PR 269584

This revision now requires review to proceed.Feb 24 2023, 4:31 PM
Harbormaster completed remote builds in B49992: Diff 117882.Feb 24 2023, 4:31 PM
rfyu28uyeg_snkmail.com added a subscriber: rfyu28uyeg_snkmail.com.Feb 24 2023, 5:36 PM

Updates look good to me. I like '#!/usr/bin/env bash' better (see comments in bug 269584, but I understand the tendency of the project toward hard-coded shebangs in most cases.

sysutils/debootstrap/Makefile
33–34

Lots of extra REINPLACE_CMD calls here. Less efficient and it triggers warnings if DEVELOPER=yes.

See also Bug 269584, comment 18 & comment 22.

rfyu28uyeg_snkmail.com added inline comments.Feb 24 2023, 5:43 PM
sysutils/debootstrap/Makefile
39

See also bug 255525 suggesting pcre[2]grep instead of gnu grep. If nothing else pcregrep is a BSD license and probably maintained a little better than gnu grep. One less gnu dependency is good. And it's probably somewhat more likely someone has devel/pcre{,2} installed than ggrep due to more dependencies on pcre than gnugrep.

Could also use 'ggrep' instead of full path with LOCALBASE/bin.

zirias marked an inline comment as done.Feb 24 2023, 5:50 PM
zirias added inline comments.
sysutils/debootstrap/Makefile
33–34

It's the same number of invocations as in your suggestion....

39

Here as well, I'm not a fan of fixing things that aren't broken. Plus, upstream is Debian, expecting GNU tools, so I'd consider using GNU grep here more robust regarding possible future changes.

rfyu28uyeg_snkmail.com added inline comments.Feb 24 2023, 6:07 PM
sysutils/debootstrap/Makefile
39

Agreed - valid point.

nc accepted this revision.Mar 7 2023, 4:49 PM

I think it looks good, feel free to commit.

This revision is now accepted and ready to land.Mar 7 2023, 4:49 PM
Closed by commit R11:4e0f740ddca6: sysutils/debootstrap: Fix several issues (authored by zirias). · Explain WhyMar 7 2023, 5:10 PM
This revision was automatically updated to reflect the committed changes.
zirias marked an inline comment as done.
zirias added a commit: R11:4e0f740ddca6: sysutils/debootstrap: Fix several issues.

Revision Contents
Changeset List

PathSize
sysutils/
debootstrap/
14 lines
files/
2 lines
2 lines
20 lines

Diff 117857

View Options

sysutils/debootstrap/Makefile

Loading...
View Options

sysutils/debootstrap/files/patch-Makefile

Loading...
View Options

sysutils/debootstrap/files/patch-debootstrap

Loading...
View Options

sysutils/debootstrap/files/patch-functions

Loading...