Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use T.workspace in Git.open() #93

Merged
merged 4 commits into from
Dec 31, 2024
Merged

Conversation

defung
Copy link

@defung defung commented Dec 24, 2024

In mill 0.12, the following line fails because the workdir (".") is now deep inside the mill-server directory, and that's not where we want to get the git repo from:

      val git            = Git.open(new File("."))

error:

om.goyeau.mill.git.GitVersionModule.version.super.GitVersionModule org.eclipse.jgit.errors.RepositoryNotFoundException: repository not found: /opt/agent/runners/0aa8062bdb914a303-4/_work/defung/test-repo/out/mill-server/5872f4ddb9601f7e0e0e05b9e73414d9af91f4ba-1/sandbox
    org.eclipse.jgit.lib.BaseRepositoryBuilder.build(BaseRepositoryBuilder.java:627)
    org.eclipse.jgit.api.Git.open(Git.java:93)
    org.eclipse.jgit.api.Git.open(Git.java:73)
    com.goyeau.mill.git.GitVersionModule$.$anonfun$version$2(GitVersionModule.scala:17)

Instead, we should be explicitly using T.workspace to get the root dir of the workspace.

@joan38
Copy link
Owner

joan38 commented Dec 26, 2024

Good catch @defung
We just need a ./mill __.style and we should be good

@defung
Copy link
Author

defung commented Dec 26, 2024

@joan38 done!

@@ -7,12 +7,12 @@ import com.goyeau.mill.scalafix.StyleModule
import de.tobiasroeser.mill.integrationtest._
import mill._
import mill.scalalib._
import mill.scalalib.api.Util.scalaNativeBinaryVersion
import mill.scalalib.api.ZincWorkerUtil.scalaNativeBinaryVersion
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Util is deprecated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case untaggedRegex(hash) =>
if (isDirty) uncommitted()
else s"${hash.take(hashLength)}$snapshotSuffix"
case _ => throw new IllegalStateException(s"Unexpected git describe output: $description")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./mill __.style doesn't like throw new..., so changed this up to use pattern matching instead of fold

@@ -61,5 +65,5 @@ object GitVersionModule extends ExternalModule {
cache.writeTree(altGit.getRepository.newObjectInserter()).abbreviate(hashLength).name()
}

override lazy val millDiscover: Discover[this.type] = Discover[this.type]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discover in mill 0.12.x no longer take a type parameter:

[130/142] mill-git[0.12.4].compile
[info] compiling 1 Scala source to /Users/dennis.fung/repos/defung/mill-git/out/mill-git/0.12.4/compile.dest/classes ...
[error] /Users/defung/repos/joan38/mill-git/mill-git/src/com/goyeau/mill/git/GitVersionModule.scala:68:35: mill.define.Discover does not take type parameters
[error]   override lazy val millDiscover: Discover[this.type] = Discover[this.type]
[error]                                   ^
[error] one error found
1 targets failed
mill-git[0.12.4].compile Compilation failed

@@ -20,7 +20,7 @@ jobs:
- name: Checks
run: |
git config --global user.name "CI"
./mill all __.checkStyle __.docJar __.test
./mill all __.checkStyle __.docJar __.publishLocal __.test
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed __.publishLocal in order for it to work with mill 0.12 for some reason

@joan38 joan38 merged commit 4314f36 into joan38:main Dec 31, 2024
1 check passed
@defung defung deleted the support_mill_0.12 branch December 31, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants