support all scala-cli dependency directives #3188
Conversation
| gitAlg | ||
| .findFilesContaining(buildRoot.repo, "//> using lib ") | ||
| ScalaCliAlg.directives | ||
| .flatTraverse(gitAlg.findFilesContaining(buildRoot.repo, _)) |
There was a problem hiding this comment.
I guess there is probably a more efficient way to do this, e.g. using one command, but I didn't spend too long looking into this
There was a problem hiding this comment.
Yeah, git grep (which is used by findFilesContaining) can search for multiple patterns in one go by combining them via --or.
I just measured the wall clock time of containsBuild before and with this change for this repo and it went from 7 to 82 milliseconds. IMO this is acceptable since it is still dwarfed by actually running any build tool to extract dependencies.
|
I updated the BuildToolDispatcherTest to account for the new searches |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3188 +/- ##
==========================================
+ Coverage 91.07% 91.12% +0.05%
==========================================
Files 163 163
Lines 3406 3404 -2
Branches 284 311 +27
==========================================
Hits 3102 3102
+ Misses 304 302 -2 ☔ View full report in Codecov by Sentry. |
bishabosha
changed the title
|
@fthomas is there something I should change about the approach? We'd like to enable scala-steward on scala/toolkit and this would help |
| "compileOnly.dep", | ||
| "compileOnly.deps", | ||
| "compileOnly.dependencies" | ||
| ).map(alias => s"//> $alias ") |
There was a problem hiding this comment.
Isn't using missing here?
| ).map(alias => s"//> $alias ") | |
| ).map(alias => s"//> using $alias ") |
There was a problem hiding this comment.
oh good point!
Edit: added a new commit
| List( | ||
| "lib", | ||
| "libs", | ||
| "dep", | ||
| "deps", | ||
| "dependencies", | ||
| "test.dependency", | ||
| "test.dep", | ||
| "test.deps", | ||
| "test.dependencies", | ||
| "compileOnly.lib", | ||
| "compileOnly.libs", | ||
| "compileOnly.dep", | ||
| "compileOnly.deps", | ||
| "compileOnly.dependencies" | ||
| ).map(alias => s"//> using $alias ") |
There was a problem hiding this comment.
@bishabosha do we need toolkit in this list as well? So we can get updates for directives like
//> using toolkit 0.2.1
There was a problem hiding this comment.
From what I could see this would need a bigger change as the algorithm searches for explicit strings so unless you want to search for all possible toolkits this isn't very flexible
There was a problem hiding this comment.
@bishabosha thanks for looking at it but I'm not sure I understand what you mean. Are you saying that adding toolkit here wouldn't match
//> using toolkit typelevel:0.1.19
why not?
There was a problem hiding this comment.
Scala Steward can already update toolkits: https://github.com/scala-steward-org/test-repo-1/pull/82/files
So I think it makes sense to add //> using toolkit here so that projects, that have no other dependencies than a toolkit, are recognized as Scala CLI projects.
I have made a list of dependency aliases from https://github.com/VirtusLab/scala-cli/blob/9e22d4a91ba8699ac2727d2ac3042d64abe951e1/modules/directives/src/main/scala/scala/build/preprocessing/directives/Dependency.scala#L33-L48
as far as I know these are not exposed programatically, but I guess they could be scraped using a source generator
fixes #3174