Page MenuHomeFreeBSD

framework: Fix fetch-url-list and fetch-urlall-list
ClosedPublic

Authored by 0mp on Jun 30 2023, 11:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 25, 12:15 PM
Unknown Object (File)
Fri, Oct 25, 12:15 PM
Unknown Object (File)
Fri, Oct 25, 12:08 PM
Unknown Object (File)
Fri, Oct 25, 12:29 AM
Unknown Object (File)
Oct 1 2024, 7:54 PM
Unknown Object (File)
Sep 28 2024, 4:46 PM
Unknown Object (File)
Sep 14 2024, 4:21 AM
Unknown Object (File)
Sep 13 2024, 7:01 PM
Subscribers

Details

Summary

The fetch-url-list and fetch-urlall-list targets are meant to produce
a list of URLs from which a port fetches its distfiles and patches.

Currently, those targets were not working as expected as they print
parts of the output meant for other targets like the fetch-list target.

For example:

mateusz.piotrowski@server /ports/x11-servers/xwayland-devel$ make fetch-urlall-list
[...]
mkdir -p "xorg" &&
-o xorg/proto-xorgproto-824001c947cb1962209c6a8f2c63c2637877220d_GL0.tar.gz
[...]

This patch prevents do-fetch.sh from printing the "mkdir" line and the
"-o" line.

While here:

  • Remove the outdated comment about escaping. There is no excessive escaping happening there anymore.
  • Indent cases in the case statement to match the style of the rest of the file.

Sponsored by: Klara Inc.

Test Plan
make -C x11-servers/xwayland-devel fetch-urlall-list
make -C accessibility/sct fetch-urlall-list

Diff Detail

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

Event Timeline

0mp requested review of this revision.Jun 30 2023, 11:59 AM

It seems you removed the case when file is not */*, but matching *.
Is it always the case that file matches */*?
If yes, why even test it? If not, shouldn't we manage it as it was before, without creating the directory?

It seems you removed the case when file is not */*, but matching *.
Is it always the case that file matches */*?
If yes, why even test it? If not, shouldn't we manage it as it was before, without creating the directory?

This is very good observation.

  1. The only thing that the * case did was set args="${site}${file}". In the patch version, this is what it ultimately done for every case. That's why I moved it outside of the case statement (see line 143 in the patched version).
  2. Regarding dropping the case statement completely: I do not think we can do it. From what I understand, $file can either contain a / or not. We only want to do the extra steps of going into the dp_TARGET case statement if $file contains a /. That is why we have to keep the $file case statement.

All in all, this patch has two main changes:

  1. Separate fetch-list from fetch-url-list-int logic, because they need different handling.
  2. Refactor the assignment of args so that we assign args once after the case statements based on the contents of $early_args, $site, and $file.

I understand.
In this case, my only observation was that we are using a case/esac construct instead of an if/fi, but the matching condition seems more natural with case/esac.

This revision is now accepted and ready to land.Jul 11 2023, 9:08 PM