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

Allow checking staged Apache artifacts #253

Merged
merged 5 commits into from
Jul 28, 2023
Merged

Conversation

raboof
Copy link
Owner

@raboof raboof commented Jul 14, 2023

messy hack still, TODO generalize so that this can be a proper feature

raboof and others added 2 commits July 14, 2023 09:49
messy hack still, TODO generalize so that this can be a proper
feature
@jrudolph
Copy link

Here's a diff that prevents rebuilding docs all the time when checking:

diff --git a/src/main/scala/ReproducibleBuildsPlugin.scala b/src/main/scala/ReproducibleBuildsPlugin.scala
index b6b3f23..98630c3 100644
--- a/src/main/scala/ReproducibleBuildsPlugin.scala
+++ b/src/main/scala/ReproducibleBuildsPlugin.scala
@@ -54,7 +54,7 @@ object ReproducibleBuildsPlugin extends AutoPlugin {
       reproducibleBuildsPackageName.value,
       version.value,
       scmInfo.value,
-      (packagedArtifacts in Compile).value,
+      artifactsWithoutDocs.value,
       (libraryDependencies in Compile).value,
       (scalaVersion in artifactName).value,
       (scalaBinaryVersion in artifactName).value,
@@ -62,6 +62,14 @@ object ReproducibleBuildsPlugin extends AutoPlugin {
     )
   }
 
+  // Since we do not care about docs and the docs task is not cached,
+  // we use a simplified version of `packagedArtifacts` that only includes jar and pom
+  // https://github.com/sbt/sbt/blob/2d7db22785726f30a95a0b2d7aea7ae5059ab51c/main/src/main/scala/sbt/Defaults.scala#L2788C3-L2791
+  // This might miss changes in other packaged artifacts that were added by the user.
+  lazy val artifactsWithoutDocs = Def.task {
+    Seq((Compile / makePom / packagedArtifact).value, (Compile / packageBin / packagedArtifact).value).toMap
+  }
+
   lazy val ourCertificationFile = Def.task[File] {
     val certification = ourCertification.value
 
@@ -122,7 +130,7 @@ object ReproducibleBuildsPlugin extends AutoPlugin {
       if (publishCertification.value) generatedArtifact else Map.empty[Artifact, File]
     },
     reproducibleBuildsCheckMavenCentral := {
-      val ourArtifacts = (packagedArtifacts in Compile).value
+      val ourArtifacts = artifactsWithoutDocs.value
       val url = artifactUrl(MavenCentral,"buildinfo").value
 
       val log = streams.value.log

@raboof raboof changed the title WIP: checking staged Apache artifacts Allow checking staged Apache artifacts Jul 27, 2023
@raboof raboof marked this pull request as ready for review July 27, 2023 14:50
@mdedetrich
Copy link
Collaborator

mdedetrich commented Jul 27, 2023

Here's a diff that prevents rebuilding docs all the time when checking:

Questioning the premise a bit, but shouldnt' we care about checking docs? Although generating docs right now is redundant because we never check the javadoc jars, is there a point in checking docs especially considering that docs can differ depending on which JDK you build them with (we have had these issues in Pekko)

@mdedetrich
Copy link
Collaborator

@raboof Would it be an issue to merge this for now and then release a new version? Its not perfect but more improvements can be done in the future and having this released would save a lot of time when checking for releases in Pekko.

@raboof
Copy link
Owner Author

raboof commented Jul 28, 2023

@raboof Would it be an issue to merge this for now and then release a new version? Its not perfect but more improvements can be done in the future and having this released would save a lot of time when checking for releases in Pekko.

that's indeed my plan, still running some tests but planning to merge/release today

@raboof
Copy link
Owner Author

raboof commented Jul 28, 2023

Here's a diff that prevents rebuilding docs all the time when checking:

would #257 also achieve that? or manually disabling the plugin for that subproject?

@raboof raboof merged commit f7608c2 into main Jul 28, 2023
@mdedetrich
Copy link
Collaborator

would #257 also achieve that? or manually disabling the plugin for that subproject?

I think this is a separate issue, @jrudolph point seems to be "don't generate docs since we don't check doc jars" where as #257 is about not redundantly checking artifacts which the sbt build doesn't publish (i.e. causing erroneous Missing in theirs but present in ours)

@raboof
Copy link
Owner Author

raboof commented Jul 28, 2023

@jrudolph point seems to be "don't generate docs since we don't check doc jars"

Ah, ofc 🤦 . If we do sign and publish doc jars, perhaps we should care about them though?

@mdedetrich mdedetrich deleted the check-staged-apache-artifacts branch July 28, 2023 09:25
@mdedetrich
Copy link
Collaborator

Ah, ofc 🤦 . If we do sign and publish doc jars, perhaps we should care about them though?

Indeed, thats my opinion as well (i.e. we should be checking the checksum of javadoc jars).

@jrudolph
Copy link

@jrudolph point seems to be "don't generate docs since we don't check doc jars"

Ah, ofc 🤦 . If we do sign and publish doc jars, perhaps we should care about them though?

Not a strong opinion about that, it just seemed wasteful to rebuild apidocs all the time when don't check them anyway.

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.

3 participants