dependency_collector: fix caching of deps requiring brewed curl

[EricFromCanada]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

This addresses a rather subtle dependency bug that's best illustrated with an example. If a formula's source needs brewed curl to be downloaded, it's marked as using: :homebrew_curl and will show curl as an implicit dependency.

$ brew deps --include-build --direct --tree --annotate libzip
libzip
├── curl  [build] [test] [implicit]
├── cmake  [build]
└── zstd 

Same if the formula is a dependent for another.

$ brew deps --include-build --tree --annotate xlsxio
xlsxio
└── libzip 
    ├── curl  [build] [test] [implicit]
    │   ├── pkg-config  [build]
...
    ├── cmake  [build]
    └── zstd 
        ├── cmake  [build]
        ├── lz4 
        └── xz 

But in some cases, curl isn't added as an implicit dependency when it should be.

$ brew deps --include-build --tree --annotate libisofs
libisofs
├── autoconf  [build]
│   └── m4 
├── automake  [build]
│   └── autoconf 
│       └── m4 
├── libtool  [build]
│   └── m4 
└── libzip 
    ├── cmake  [build]
    └── zstd 
        ├── cmake  [build]
        ├── lz4 
        └── xz 

In the above example, removing autoconf/automake/libtool will make curl show properly as a libzip dependency. So what's going on?

In #12296, I included a dirty workaround for the possibility that a resource requiring brewed curl to download (i.e. using HomebrewCurlDownloadStrategy) may also require another implicit dependency to open the downloaded file, which was to add curl directly to the dependency list with @deps << brewed_curl_dep_if_needed(tags), and then proceed with analyzing the URL as is done with the regular CurlDownloadStrategy. The problem is that Dependency objects are cached. So, if a formula using e.g libzip (which is a .xz file) also depends on another formula which is the same file type (e.g. libtool, also a .xz file) and that dependency is processed first, the cache says that no implicit dependencies are required for this file type, causing the above code to be skipped.

The fix is to 1) use a separate cache key for HomebrewCurlDownloadStrategy dependency objects, and 2) instead of inserting into @deps directly, send multiple items back to add() and insert them there.

[Bo98]

Makes sense, thanks!

[Bo98]

I wonder if the Resource case could be generalised to spec.download_strategy (+ File.extname(spec.url) in the curl case), given all of the other download strategies look like they could also be cached. Not sure if any speedup would be noticeable

[EricFromCanada]

Yep, seems to work. Also added a case for VCSDownloadStrategy which didn't exist when the original method was written.

[Bo98]

LGTM

[Bo98]

Do we need the condition at all? Would this work?

Suggested change

	elsif spec.download_strategy <= VCSDownloadStrategy
	else

[EricFromCanada]

It should if it's a safe assumption that any Resource will have a download strategy defined.

[MikeMcQuaid]

Thanks @EricFromCanada, good catch!