Skip to content

Conversation

@LizBaldo
Copy link
Contributor

@LizBaldo LizBaldo commented Dec 4, 2025

Duplicate of this PR #7825

Making a copy here so we don't merge from a different fork, and can use the full CI suite.

@sberss
Copy link

sberss commented Dec 5, 2025

I seem to have made a mistake in this PR, I included both the changes that we were originally hacking in to work only with our use-case, alongside those for fixing this "properly". All the changes in dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala shouldn't have been included.

Patch below:

diff --git a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala
index fa5aec46d..a20e6e43c 100644
--- a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala
+++ b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala
@@ -18,7 +18,7 @@ sealed trait DockerImageIdentifier {
   lazy val name = repository map { r => s"$r/$image" } getOrElse image
   // The name of the image with a repository prefix if a repository was specified, or with a default repository prefix of
   // "library" if no repository was specified.
-  lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"$image"
+  lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"/$image"
   lazy val hostAsString = host map { h => s"$h/" } getOrElse ""
   // The full name of this image, including a repository prefix only if a repository was explicitly specified.
   lazy val fullName = s"$hostAsString$name:$reference"
@@ -59,7 +59,7 @@ case class DockerImageIdentifierWithHash(host: Option[String],

 object DockerImageIdentifier {
   private val DefaultDockerTag = "latest"
-  val DefaultRepository = ""
+  val DefaultRepository = "library"

   private val DockerStringRegex =
     s"""

Sorry!

@LizBaldo
Copy link
Contributor Author

LizBaldo commented Dec 5, 2025

DockerImageIdentifier

No worries, I was about to reach out about that be you beat me to it :) I am sorry you are unable to trigger the integration tests fro you original branch that would have made your job a lot easier. Appreciate the help!

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.

5 participants