Remove recursion from Complete.hasOrg #2655
Conversation
This is primarily to fix an issue where the Sonatype snapshots repository disallows listing the top-level directory, but the recursion appears to have no functional benefit. This should also make complete faster by reducing the number of queries.
|
I also suspect that the |
| case true => | ||
| check | ||
| } | ||
| ): F[Boolean] = |
There was a problem hiding this comment.
I recommend hiding whitespace on the diff to see that this is just taking the expression assigned to check and returning it.
|
This isn't a complete fix for compete on sonatype snapshots. Complete still doesn't work for organizations, but it works for artifacts and versions which seems like a pretty big improvement to me. I proved this worked to myself with the following script (running it with and without this change): #!/usr/bin/env bash -x
CS='mill -s --disable-ticker -i cli.run'
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu.berkeley.cs:chisel3_2.12:3.2
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu.berkeley.cs:chisel3_2.12:
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu.berkeley.cs:chisel3
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu.berkeley.cs:
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu.berkeley
$CS complete-dep --cache localcache --no-default -r sonatype:snapshots edu
$CS complete-dep --cache localcache --no-default -r sonatype:releases edu.berkeley.cs:chisel3_2.12:3.2
$CS complete-dep --cache localcache --no-default -r sonatype:releases edu.berkeley.cs:chisel3_2.12:
$CS complete-dep --cache localcache --no-default -r sonatype:releases edu.berkeley.cs:chisel3
$CS complete-dep --cache localcache --no-default -r sonatype:releases edu.berkeley.cs:
$CS complete-dep --cache localcache --no-default -r sonatype:releases edu.berkeley
$CS complete-dep --cache localcache --no-default -r sonatype:releases eduYou'll notice that |
There was a problem hiding this comment.
Thanks @jackkoenig!
Do you mind if I ponder that a little? IIRC, this recursion was related to caching, with things being properly cached with the recursion, and not correctly cached without it. I don't recall if this still applies though, which what I'd like to check…
|
Sorry I missed your response @alexarchambault! Yeah this isn't urgent by any means but I hope some solution eventually makes its way into coursier! Thanks for the great tool 🙂 |
This is primarily to fix an issue where the Sonatype snapshots repository disallows listing the top-level directory, but the recursion appears to have no functional benefit. This should also make complete faster by reducing the number of queries.
Fixes #1700
Many thanks and full credit to @keynmol for doing the real legwork debugging the issue in #1700 (comment).
I'm opening this as a draft PR because I am not sure how best to test this, especially since it relies on peculiarities of the Sonatype snapshots repository, and my limited grepping did not find any tests for the behavior of Complete.