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

Feature: java_library.resource_add_prefix #6683

Closed
BlackIsTheNewBlack opened this issue Nov 15, 2018 · 10 comments
Closed

Feature: java_library.resource_add_prefix #6683

BlackIsTheNewBlack opened this issue Nov 15, 2018 · 10 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: feature request

Comments

@BlackIsTheNewBlack
Copy link

Description of the feature request:

java_library has a resource_strip_prefix property which removes a path prefix. Please add the complementary property "resource_add_prefix" which would place files in a different folder.

Feature requests: what underlying problem are you trying to solve with this feature?

Many times resources need to be placed in specially named folders in order to work. For example, Java ServiceLoader resources have to be placed under META-INF/services. Ideally, in the source folders the resource file would be placed right near the {Service}.java it describes. Currently that is not possible, it has to be either placed in a META-INF/services subfolder (which increases the complexity of the directory structure) or in a dedicated resources/META-INF/services folder which aggreggates all service resource, which breaks modularity.

Without resource_add_prefix:
Service.java
META-INF/
services/
com.test.Service

With proposed resource_add_prefix='META-INF/services':
Service.java
com.test.Service

What operating system are you running Bazel on?

Windows 10

What's the output of bazel info release?

release 19.1

Have you found anything relevant by searching the web?

Nothing

@cushon
Copy link
Contributor

cushon commented Nov 16, 2018

ServiceLoader resources have to be placed under META-INF/services

Consider using AutoService for this. It's an annotation processor that generates the META-INF/services files for you.

Currently that is not possible

It's not possible using attributes on java_library, but if it's necessary to have full control over the resource layout you can generate a separate .jar, add it to the build using java_import, and then add the java_import to the deps or runtime_deps of the original java_library.

@cushon cushon added the team-Rules-Java Issues for Java rules label Nov 16, 2018
@ittaiz
Copy link
Member

ittaiz commented Nov 16, 2018 via email

@BlackIsTheNewBlack
Copy link
Author

@ittaiz
Thank you for the tip. Please notice that the zipper is not documented and requires two extra rules to manage. The same argument could be made for strip_resource_prefix, why have it when you can just merge an external archive?

@cushon
Actually I did not need it for ServiceLoader, it was just an example, I need it for a Vaadin web app, which actually requires 3 extra directory levels (META-INF/resources/frontend) and webjars (META-INF/resources/webjars/iron-...) . IMHO the java_library rule should be self contained if possible, strip_resource_prefix is half usable without its complementary resource_add_prefix.

Thank you for your time.

@cushon
Copy link
Contributor

cushon commented Nov 19, 2018

The same argument could be made for strip_resource_prefix, why have it when you can just merge an external archive?

I think it's worth considering deprecating resource_strip_prefix. @lberki do you remember the motivation for beccb9b? I wonder if it was to work around issues like #6353?

IMHO the java_library rule should be self contained if possible

I don't see this is a goal. I'd rather have slightly more rules that solve specific problems than having java_library try to do everything.

@ittaiz
Copy link
Member

ittaiz commented Nov 20, 2018 via email

@BlackIsTheNewBlack
Copy link
Author

BlackIsTheNewBlack commented Nov 20, 2018

Hello,

I think it's worth considering deprecating resource_strip_prefix.

Please do not deprecate resource_strip_prefix unless you provide a mechanism for controlling the paths layouts in the resulting jar, which should at least be able to... strip the prefix. The layout of resources in the JARs has to be separated from the logical layout of source files and resource file, because they are guided by different requirements.

IMHO the java_library rule should be self contained if possible
I don't see this is a goal. I'd rather have slightly more rules that solve specific problems than having java_library try to do everything.

How about this:
Explicitely map the layout of resources with a map of labels and some regex magic using capturing groups:

$STRIP='//some/embedding/root'

META_ROOT = 'META-INF/resources/frontend'

resources = glob(
{
"$STRIP/(**)/(*).properties.in": i18n("$META_ROOT/$1/$LANG/$2.properties"),
} )

In this example, the resource prefix is stripped, the destination root is changed, same for file extension, and it is also using custom logic for the localization language.

This approach can replace java_library.resources, resource_strip_prefix and classpath_resources properties, which would become obsolete, along with their limitations such as "This heuristic cannot be overridden.". #6353 can also be closed because the system wide configuration can be replaced with a specific map and some function calls.

In order to address the IO concern raised by @ittaiz, please consider adding a rule equivalent with rsync which would operate directly on such a map of source files to destination files, based in a dedicated directory instead of a zip.

@cushon
Copy link
Contributor

cushon commented Nov 20, 2018

Aren’t you concerned about the additional IO? One needs to unzip and zip.

The implementation of java_library.resources creates a separate action to add the resources to the output jar, to avoid re-running the compilation action if only the resources change. Creating a completely separate jar for the resources should be marginally less expensive than merging the resources into the library's output jar. And either way, creating the zip is almost free compared to the cost of compilation.

a mechanism for controlling the paths layouts in the resulting jar

I think the approach described earlier in the bug provides full control over the resource layout? #6683 (comment)

@BlackIsTheNewBlack
Copy link
Author

I think the approach described earlier in the bug provides full control over the resource layout?

In my opinion users of Bazel should not be expected to program the build system in order to achieve common results. Composing sets of build outputs into a precise dir layout is a generic operation which deserves its own rule. It would greatly simplify the treatment of jar resources and web site builds in general.

My previous comment (#6683 (comment)) was to describe how such a layout mapping rule should look to the user. It is just a map of search,replace pairs, ie a function from the old path to the new path. Should the java_library rule depend on such a file_mapping rule it could replace the resource_strip_prefix and the proposed resource_add_prefix.

@lberki lberki added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 14, 2019
@comius comius added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: feature request and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Nov 21, 2020
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 12, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants