Repro for Duplicated item inserted into OrderedSet #2958
Milestone
Comments
|
Thanks for the reproducer! |
lefou
added a commit
that referenced
this issue
Jan 8, 2024
Ensure when flatMap-ing, that we don't append duplicates. Whether this is correct needs yet to be analyzed, but in general, we don't enfore unique-ness of task results, and even when we evaluate disjuct tasks, we can't assume that all results are disjuct, hence using a strict Agg, which enforces uniqueness on append seems not correct. Here's my reasoning: If we have two tasks A and B, and B just forwards the result of A as it's own result, both tasks return the same result.
|
@Quafadas I have a quick fix for you to try. It should solve your issue, although the solution should be discusses and improved. |
|
Er - blimey. that as quick. Once the CI goes though I'll try and track down the hash, and take it for spin :-)... Cool - thanks! |
|
Minimized reproduction: // file: scratch/build.sc
import mill._, mill.scalalib._
import mill.define.ModuleRef
val repos = T.task {
Seq(
coursier.maven.MavenRepository("https://repo1.maven.org/maven2"),
coursier.LocalRepositories.ivy2Local
)
}
object CustomZincWorkerModule extends ZincWorkerModule with CoursierModule {
def repositoriesTask = repos
}
object foo extends JavaModule {
def repositoriesTask = repos
def zincWorker = ModuleRef(CustomZincWorkerModule)
} |
|
I guess, this issue is, that the anonymous task And indeed, if I change |
lefou
added a commit
that referenced
this issue
Jan 9, 2024
lefou
added a commit
that referenced
this issue
Jan 10, 2024
Anonymous tasks can be defined as `val` or `def`. This is in contrast to named targets, which always need to be defined as `def`. Also, anonymous tasks end up in the terminal group of the target that's using them. If the same anonymous task is used in multiple targets, it also ends up multiple time in the task results. When a `Evaluator.Results` is created, this clashes, due to the use of a `Strict.Agg` for `Evalutor.Results.evaluated`. This change deduplicates the results before constructing the `Evaluator.Results`. Pull request: #2959
|
@lefou thankyou! Looked a little more painful than expected... but👏, very excited to try it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@lefou Aha! I believe I have a semi-minimal reproduction for this on a public repository.
git clone https://github.com/Quafadas/vecxt.gitmill -j 1 vecxt.jvm.prepareOfflinegit checkout build_errormill cleanmill -j 1 vecxt.jvm.prepareOfflineThe first time I do this, I get
Second and subsequent incarnations of
vecxt.jvm.prepareOfflinesucceed, I suspect because something is correctly cached.I can reproduce, by cleaning and re-running the command. It's also with
-j 1, so I hope easier to work with.I now suspect, it can be minimised further, but that's the best I have for right now - I'll have a look at something truly minimal in the coming days, if I can... but I can't see a reason this shouldn't be the same for anyone else, too.
The text was updated successfully, but these errors were encountered: