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

Plugin not always triggers scala classes rebuild. #307

Closed
fido-node opened this issue May 3, 2022 · 5 comments
Closed

Plugin not always triggers scala classes rebuild. #307

fido-node opened this issue May 3, 2022 · 5 comments

Comments

@fido-node
Copy link

Steps to reproduce.

git clone [email protected]:michey/sbt-example.git
cd sbt-example
git checkout 1.0.0-branch
cd external-iface 
sbt publishLocal
git checkout 2.0.0-branch
sbt publishLocal

Successful case:

cd ../app
git checkout 1.0.0-branch
sbt compile
# works good. (expected result)

Failed case

git checkout 2.0.0-branch
sbt compile
# also works. (unexpected result)

Expected result:
Compilation in 2.0.0-branch should fails because proto file has one less field and scala code tries to call constructor with two parameters.

Known workaround:

git checkout 2.0.0-branch
sbt clean
sbt compile

Diff between 1.0.0-branch and 2.0.0-branch

diff --git a/app/build.sbt b/app/build.sbt
index 01e2f26..6a5a63a 100644
--- a/app/build.sbt
+++ b/app/build.sbt
@@ -18,7 +18,7 @@ lazy val exampleProtobuf =
       PB.protocVersion := "3.17.3",
       libraryDependencies ++= Seq(
         "com.thesamet.scalapb" %% "scalapb-runtime" % scalapb.compiler.Version.scalapbVersion % "protobuf",
-        "acme" %% "external-iface" % "1.0.0" % "protobuf-src" intransitive(),
+        "acme" %% "external-iface" % "2.0.0" % "protobuf-src" intransitive(),
       ),
       Compile / PB.targets := Seq(
         scalapb.gen() -> (Compile / sourceManaged).value
diff --git a/external-iface/build.sbt b/external-iface/build.sbt
index faabb74..4af7277 100644
--- a/external-iface/build.sbt
+++ b/external-iface/build.sbt
@@ -2,4 +2,3 @@ name := "external-iface"
 scalaVersion := "2.13.7"
 organization := "acme"
 Compile / unmanagedResourceDirectories += baseDirectory.value / "protobuf"
-
diff --git a/external-iface/protobuf/iface/experimentation_service.proto b/external-iface/protobuf/iface/experimentation_service.proto
index 126d105..904c963 100644
--- a/external-iface/protobuf/iface/experimentation_service.proto
+++ b/external-iface/protobuf/iface/experimentation_service.proto
@@ -5,5 +5,5 @@ option java_package = "iface";
 
 message Message {
   string first_field = 1;
-  string second_field = 2;
+//  string second_field = 2;
 }
diff --git a/external-iface/version.sbt b/external-iface/version.sbt
index 4b6a8b5..b547370 100644
--- a/external-iface/version.sbt
+++ b/external-iface/version.sbt
@@ -1 +1 @@
-ThisBuild / version := "1.0.0"
\ No newline at end of file
+ThisBuild / version := "2.0.0"
\ No newline at end of file

Summary: As I can see, sbt updates jar with proto files, sbt-protoc plugin unpacks this proto file in the right place, but plugin do not regenerate scala classes.

@fido-node
Copy link
Author

fido-node commented May 8, 2022

I've dug a little bit deeper and found that this problem is not related to sbt-protoc plugin.
This behavior is related to these changes in sbt. sbt/sbt#5344
sbt devs decide to implement "reproducible builds" through setting all mtime in jars to the fixed value. After the plugin unarchives these sources from jars we get protoc files with the same date for different versions.
If you do not need this feature in your build pipelines you can easily opt-out
sbt/sbt#6237
You still have a chance to meet this problem if you have protoc files with the same mtime across different versions. But the chance is extremely low. ¯\_(ツ)_/¯

TL;DR: If your scala classes are not updating after changing the dependency version try to add ThisBuild / packageTimestamp := Package.keepTimestamps to your build.sbt on the publishing side.

@notxcain
Copy link

notxcain commented May 9, 2022

I'm not sure that solving this issue on the publisher side is a good idea. @thesamet WDYT?

@fido-node
Copy link
Author

Since we have a new questions about this fix, I reopen this issue.

@fido-node fido-node reopened this May 30, 2022
@thesamet
Copy link
Owner

thesamet commented May 30, 2022

Agree that relying on the publisher isn't ideal since it's possible we have no control over the publisher's build. Maybe it's possible to somehow cache the protobuf-src libraryDependencies into the Arguments object so changing the dependencies would invalidates the build. Happy to review a PR for that or some other way to detect a new jar was unpacked. Would rather avoid the hashing solution, since then all users (who mostly aren't impacted by this issue) pay the penalty of hashing all proto files.

@thesamet
Copy link
Owner

It looks like there's a reasonable workaround and there wasn't any activity on this issue for quite a while, so closing.

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

No branches or pull requests

3 participants