-
-
Notifications
You must be signed in to change notification settings - Fork 340
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
Favor -SNAPSHOT version when publishing Mill locally #3615
base: main
Are you sure you want to change the base?
Conversation
This makes the millVersion task return a more stable value. That value ends up in BuildInfo files, so having it not change prevents it trigerring re-compilations. Without this change, adding files to the staging area, committing things, or sometimes just creating new files, was changing the millVersion, and as a consequence trigerring a re-compilation of many things.
Don't know if you rely locally on the more detailed versions. They're really a loss of time for me. |
@lefou could you help take a look at this? All this code is interacting with the vcs version and you probably have thought about it more than I have |
Sure, we can change the configuration. In a work project, we instead use a static VcsVersion.vcsState().format(dirtyHashDigits = 0) In the same project, we also have a local override. def staticVersion: T[Option[String]] = T {
val file = staticVersionFile().path
if (os.exists(file)) {
val version = Option(os.read(file).trim()).filter(_.nonEmpty)
T.log.info(s"Using static version `${version}` from file: ${file}")
version
} else None
}
def version: T[String] = T {
val git = VcsVersion.vcsState().format(dirtyHashDigits = 0, commitCountPad = 4, countSep = "-")
staticVersion().getOrElse(git)
} |
def millVersion: T[String] = T { | ||
val vcsVersion = VcsVersion.vcsState().format() | ||
val isCI = System.getenv("CI") != null | ||
if (!isCI && (vcsVersion.contains("DIRTY") || vcsVersion.count(_ == '-') >= 2)) { | ||
// vcsVersion looks like "0.11.11-6-49d084-DIRTYa0693949", we only keep | ||
// the "0.11.11" part here | ||
val baseVersion = vcsVersion.takeWhile(_ != '-') | ||
val lastDotIndex = baseVersion.lastIndexOf('.') | ||
if (lastDotIndex < 0) | ||
// unexpected shape, keeping vcsVersion | ||
vcsVersion | ||
else { | ||
val patchVersionOpt = baseVersion.substring(lastDotIndex + 1).toIntOption | ||
patchVersionOpt match { | ||
case None => | ||
// unexpected shape, keeping vcsVersion | ||
vcsVersion | ||
case Some(patchVersion) => | ||
val base = baseVersion.substring(0, lastDotIndex) | ||
s"$base.${patchVersion + 1}-SNAPSHOT" | ||
} | ||
} | ||
} | ||
else | ||
vcsVersion | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might misreading this longish blob, but I guess what you want is:
def millVersion: T[String] = T { | |
val vcsVersion = VcsVersion.vcsState().format() | |
val isCI = System.getenv("CI") != null | |
if (!isCI && (vcsVersion.contains("DIRTY") || vcsVersion.count(_ == '-') >= 2)) { | |
// vcsVersion looks like "0.11.11-6-49d084-DIRTYa0693949", we only keep | |
// the "0.11.11" part here | |
val baseVersion = vcsVersion.takeWhile(_ != '-') | |
val lastDotIndex = baseVersion.lastIndexOf('.') | |
if (lastDotIndex < 0) | |
// unexpected shape, keeping vcsVersion | |
vcsVersion | |
else { | |
val patchVersionOpt = baseVersion.substring(lastDotIndex + 1).toIntOption | |
patchVersionOpt match { | |
case None => | |
// unexpected shape, keeping vcsVersion | |
vcsVersion | |
case Some(patchVersion) => | |
val base = baseVersion.substring(0, lastDotIndex) | |
s"$base.${patchVersion + 1}-SNAPSHOT" | |
} | |
} | |
} | |
else | |
vcsVersion | |
} | |
def millVersion: T[String] = T { | |
VcsVersion.vcsState().format(dirtySep = "-SNAPSHOT", dirtyHashDigits = 0) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be ok with I do use But For this PR that seems like it will speed up |
The pattern I was proposing is |
I tend to create small commits and rebase a lot during development (to make sense of the changes, even though I only push incremental "fixup" commits now 😅), so |
@alexarchambault What about overriding the version with the content of some optional file or env var? I already posted it above. |
@lefou Sorry, I missed that. That would work too, yes. |
This makes the millVersion task return a more stable value during local development (the version on CI doesn't change on the other hand). That value ends up in BuildInfo files, so having it not change prevents it trigerring re-compilations.
Without this change, adding files to the staging area, committing things, or sometimes just creating new files, was changing the millVersion, and as a consequence trigerring a re-compilation of many things.