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

Not assembly the pekko-protobuf-v3... everytime? #1008

Closed
Tracked by #1052
He-Pin opened this issue Jan 21, 2024 · 10 comments
Closed
Tracked by #1052

Not assembly the pekko-protobuf-v3... everytime? #1008

He-Pin opened this issue Jan 21, 2024 · 10 comments
Labels
build sbt or build of the project help wanted Extra attention is needed

Comments

@He-Pin
Copy link
Member

He-Pin commented Jan 21, 2024

The pekko-protobuf-v3... is packaged everytime,you run a test, Is is possible not repackage it if the sbt session is not reloaded?

[IJ]all {file:/C:/Users/hepin/IdeaProjects/incubator-pekko/}stream-tests/products {file:/C:/Users/hepin/IdeaProjects/incubator-pekko/}stream-tests/test:products
[info] 3 file(s) merged using strategy 'Rename' (Run the task at debug level to see the details)
[info] 3 file(s) merged using strategy 'Discard' (Run the task at debug level to see the details)
[info] Built: C:\Users\hepin\IdeaProjects\incubator-pekko\protobuf-v3\target\scala-2.13\pekko-protobuf-v3-assembly-1.1.0-M0+255-637d72af-SNAPSHOT.jar
[info] Jar hash: c5f34e5fd0c6881b0e6a1d2d7ac9a60c097dd581
[success] Total time: 11 s, completed 2024-1-21 16:32:09
[IJ]
[IJ]all {file:/C:/Users/hepin/IdeaProjects/incubator-pekko/}stream-tests/products {file:/C:/Users/hepin/IdeaProjects/incubator-pekko/}stream-tests/test:products
[info] 3 file(s) merged using strategy 'Rename' (Run the task at debug level to see the details)
[info] 3 file(s) merged using strategy 'Discard' (Run the task at debug level to see the details)
[info] Built: C:\Users\hepin\IdeaProjects\incubator-pekko\protobuf-v3\target\scala-2.13\pekko-protobuf-v3-assembly-1.1.0-M0+255-637d72af-SNAPSHOT.jar
[info] Jar hash: c5f34e5fd0c6881b0e6a1d2d7ac9a60c097dd581
[success] Total time: 10 s, completed 2024-1-21 16:32:42
@He-Pin He-Pin added the build sbt or build of the project label Jan 21, 2024
@mdedetrich
Copy link
Contributor

Its possible, but I would consider this low priority as it may be quite tricky to figure out

@He-Pin He-Pin added help wanted Extra attention is needed high priority required for next release labels Jan 21, 2024
@Roiocam
Copy link
Member

Roiocam commented Jan 26, 2024

Based my what I know these days, I think this because sbt-assembly intercepts the compile step, and then adds itself as a post-processor, so every time you call the compile task, it will assemble again.

The correct assembly task should only run on the package task, it may be another issue of sbt-osgi, I am not gonna blame this, but the execution ofsbt-osgi has caused many problems.

@mdedetrich
Copy link
Contributor

Based my what I know these days, I think this because sbt-assembly intercepts the compile step, and then adds itself as a post-processor, so every time you call the compile task, it will assemble again.

The correct assembly task should only run on the package task, it may be another issue of sbt-osgi, I am not gonna blame this, but the execution ofsbt-osgi has caused many problems.

I just realized that sbt-osgi's caching is not enabled by default so you can maybe try enabling it?

If we do this we should be really careful and make sure there aren't any regressions

@Roiocam
Copy link
Member

Roiocam commented Jan 29, 2024

@mdedetrich @He-Pin. Got a good news, I just figured out why this happened.

Continue #1076 works, I have enabled the caching option of sbt-osgi, but it doesn't work. (But osgiBundle not running anymore, for osgi products caching, it works.)

After some digging, I found that the culprit is these lines of code.

https://github.com/apache/incubator-pekko/blob/e4ad151a28cba2e844150e16b1944740badec91f/project/Protobuf.scala#L40-L41

Let me explain this:: When other modules apply this Protobuf.settings, it will detect unmanaged dependencies during the compile step. And then finds that "assembly" has no products, so it always executes it. After executing once, it won't execute anymore.

Currently, I haven't found any other way to replace this approach.

@mdedetrich
Copy link
Contributor

After some digging, I found that the culprit is these lines of code.

https://github.com/apache/incubator-pekko/blob/e4ad151a28cba2e844150e16b1944740badec91f/project/Protobuf.scala#L40-L41

I really dislike putting IDE specific hacks into the build because it causes issues like this, can we just completely remove it?

@Roiocam
Copy link
Member

Roiocam commented Jan 29, 2024

I really dislike putting IDE specific hacks into the build because it causes issues like this, can we just completely remove it?

I have tried, but the assembly still runs. I suspect that the execution time of osgiBundle is too short for me to detect.

The assembly dependency tree shows that it can only be triggered by osgiBundle, it may also be related to the cache detection of osgiBundle. Although bnd is not executed, the jar will still be exported.

@mdedetrich
Copy link
Contributor

mdedetrich commented Jan 29, 2024

I have tried, but the assembly still runs. I suspect that the execution time of osgiBundle is too short for me to detect.

The assembly dependency tree shows that it can only be triggered by osgiBundle, it may also be related to the cache detection of osgiBundle. Although bnd is not executed, the jar will still be exported.

Thanks but here comes the next question, is Compile / unmanagedJars += (LocalProject("protobuf-v3") / assembly).value actually doing anything useful even if its not making a difference in speed? Is it even a problem with Intellij today (remember this is a 14 year old codebase so it may have been a problem with some ancient Intellij version)?

@He-Pin
Copy link
Member Author

He-Pin commented Jan 29, 2024

Or we can split this jar into a dedicated project, nad publish it to maven, as we did not expect to change it very much.

@Roiocam
Copy link
Member

Roiocam commented Jan 29, 2024

Thanks but here comes the next question, is Compile / unmanagedJars += (LocalProject("protobuf-v3") / assembly).value actually doing anything useful even if its not making a difference in speed? Is it even a problem with Intellij today (remember this is a 14 year old codebase so it may have been a problem with some ancient Intellij version)?

Yes, it will package protobuf-v3 when sbt reads and applies the build settings, and then the IDE will detect it as an unmanaged dependency. see org.apache.pekko.remote.protobuf.v3.ProtobufProtocolV3

https://github.com/apache/incubator-pekko/blob/1b0ee8cd756322ef76b1287f3e0d9067916084b2/remote/src/test/java/org/apache/pekko/remote/protobuf/v3/ProtobufProtocolV3.java#L22-L27

@mdedetrich
Copy link
Contributor

I see, so it being an unmanagedDependency is technically correct even if its not required for packaging.

@He-Pin He-Pin removed the high priority required for next release label Jan 29, 2024
@Roiocam Roiocam closed this as completed Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build sbt or build of the project help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants