Page MenuHomeFreeBSD

Mk/Scripts/do-fetch.sh: read distfiles from stdin to avoid ARG_MAX
Needs ReviewPublic

Authored by mat on Mon, Sep 7, 8:06 AM.
Tags
None
Referenced Files
F171554987: D59475.id186369.diff
Fri, Sep 11, 5:55 PM
F171532317: D59475.diff
Fri, Sep 11, 3:21 PM
F171493793: D59475.id186351.diff
Fri, Sep 11, 11:17 AM
F171492448: D59475.id186358.diff
Fri, Sep 11, 11:07 AM
F171489695: D59475.diff
Fri, Sep 11, 10:38 AM
F171484897: D59475.id186350.diff
Fri, Sep 11, 9:49 AM
F171475994: D59475.id186368.diff
Fri, Sep 11, 8:19 AM
F171472583: D59475.id.diff
Fri, Sep 11, 7:46 AM
Subscribers

Details

Reviewers
bapt
Summary

Stop passing DISTFILES and MASTER_SITES through command line arguments to avoid reaching ARG_MAX in command lines.

PR: 295029

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 76624
Build 73507: arc lint + arc unit

Event Timeline

bapt requested review of this revision.Mon, Sep 7, 8:06 AM
bapt created this revision.

I'm not sure this would fix the command line length, it it just moves the length around.
I've been thinking of a real fix for this, and I think it needs to put it all in a temporary file, kinda

.   for d in ${DISTFILES}
     printf "%s\n" "$d" >> temp
.   endfor

and then pass maybe either pass the tempfile name to do-fetch, or use it as its stdin.

Also, there is the same problem for each script that we pass the distfiles to, like makesum.

Maybe using \0 instead of \n is better there, and juggling with IFS to split on \0, in case some distfile has a strange file name.

yesh I pondered the tempfile, but this was longer, actually, but yes I think you are right, as printf will complain also about the ARG_MAX.

mat updated this revision to Diff 186349.
mat added a reviewer: bapt.

As a proof of concept, I only changed the DISTFILES part of do-fetch.

I refactored the 3 targets that had the exact same content. It's missing makesum, which will have the same problem.

makesum will need a re-design, thinking about it.

This revision is now accepted and ready to land.Thu, Sep 10, 9:43 AM
This revision now requires review to proceed.Thu, Sep 10, 9:43 AM

I may have "fixed" makesum, but maybe not, and I do not seem to find a port to test things.

Probably fix makesum and checksum

Simplify a bit, no functionnal change