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

phase_merge_jars: refactor to separate function that merges jars to o… #1403

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

gergelyfabian
Copy link
Contributor

…utput

Description

Add a new function that is independent of runtime jars and the specific output.

Motivation

This is a small refactoring as an intermediate step for possible extensions for creating
custom outputs. Not sure yet how to add those extensions, but such a refactoring here may be very useful.

An example how this can be changed further:

https://github.com/gergelyfabian/rules_scala/compare/refactor_phase_merge_jars...gergelyfabian:external-and-internal-deploy-jar?expand=1

Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gergelyfabian for the PR.

Have a separate function makes total sense. However this makes merge_jars_to_output essentially public (despite private word in its file path). This will become part of the api we will have to maintain.

I would rather wait for your anticipated change and see from there if this is needed or not. Maybe other route will be taken.

Having said that I don't want to block this PR. I just want a second opinion on this. Maybe I'm totally wrong and missing something.

Maybe this needs to be moved into separate package like starlark-wrappers-for-bazel-tools. Ideally this could come from one of the bazel modules like java_common.run_ijar.

@gergelyfabian
Copy link
Contributor Author

Here is the intended usage: gergelyfabian@0cd31de

We had a usecase, that we wanted to generate fat jars with "internal" and "external" dependencies separately (to be able to package them separately for caching reasons). Wondering what route should be optimal for this. We were also considering to add custom phases from our own application (we could reuse part of this code then, I guess).

@gergelyfabian gergelyfabian force-pushed the refactor_phase_merge_jars branch from 3446869 to bc90239 Compare November 2, 2022 12:39
…utput

This is an intermediate step for possible extensions for creating
custom outputs.
@gergelyfabian gergelyfabian force-pushed the refactor_phase_merge_jars branch from bc90239 to dd9d2b0 Compare November 23, 2022 21:24
@liucijus
Copy link
Collaborator

@gergelyfabian do you still need or use this change?

@gergelyfabian
Copy link
Contributor Author

Yes, we do all the time. We produce 3 jars for each scala_library we have and put them in 3 separate docker layers (i.e. we don't want external dependency jars - that change infrequently - land in the same layer as our internal deps - that change frequently).

@liucijus liucijus merged commit 3e3fdda into bazelbuild:master Dec 16, 2022
@liucijus
Copy link
Collaborator

Sorry for taking so long, @gergelyfabian

@gergelyfabian gergelyfabian deleted the refactor_phase_merge_jars branch December 16, 2022 12:24
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.

None yet

3 participants