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
F102850596: D38754.id.diff
Sun, Nov 17, 11:50 PM
F102832198: D38754.id.diff
Sun, Nov 17, 6:19 PM
F102819183: D38754.diff
Sun, Nov 17, 2:36 PM
Unknown Object (File)
Thu, Nov 14, 11:37 PM
Unknown Object (File)
Sun, Nov 10, 8:31 PM
Unknown Object (File)
Sun, Nov 10, 5:52 PM
Unknown Object (File)
Thu, Nov 7, 9:11 AM
Unknown Object (File)
Tue, Nov 5, 11:34 AM
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 49985
Build 46877: 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
35–36

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
43

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
35–36

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

43

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
43

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/
10 lines
files/
2 lines
2 lines
20 lines

Diff 117856

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...