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

BUILD files to build Gson with Bazel and easily depend on it in Bazel projects #2150

Closed
wants to merge 2 commits into from

Conversation

pawelz
Copy link

@pawelz pawelz commented Jul 24, 2022

Gson builds cleanly without any external dependencies, so Bazel BUILD files are very simple. I believe this won't add much maintanance burden, but will make it very easy to use Gson in Bazel projects.

This will make it easy to depend on GSON in projects using Bazel as a
build system.
@Marcono1234
Copy link
Collaborator

Marcono1234 commented Jul 24, 2022

but will make it very easy to use Gson in Bazel projects

Is this about building Gson from source using Bazel? Because to me it looks like using Gson (respectively any Maven artifact?) is already possible through rules_jvm_external.

I am not really familiar with Bazel, but shouldn't the BUILD file be at the repository root, or at least in the gson subfolder, and not in the sources package? (See Migrating from Maven to Bazel)

Unfortunately it looks like the Bazel build is not complete:

  • module-info.class seems to be missing
    (might not be so important, but certainly a minor annoyance)
  • GsonBuildConfig does not seem to be built properly (or I am using Bazel wrongly)
    The compiled classes using GsonBuildConfig.VERSION just seem to contain the placeholder value:
    throw new IllegalArgumentException("GSON (${project.version}) cannot handle " + type);
    (decompiled from Gson.class in final JAR)

Note that this project originally also had a Gradle build script but it was removed (#2063) because it became outdated, and maintaining two separate build scripts was likely not worth the effort.

I am a little bit skeptical whether Bazel build support really adds much value. However, I am not a member of this project and they might think differently about this.

@pawelz
Copy link
Author

pawelz commented Jul 24, 2022

Thank you for taking a look and commenting.

After thinking this over, you are probably right. I can work with my own fork and this is probably not worth merging in.

I will leave it open for now, in case the project owners want this change. But if they say they are also skeptical, I'll withdraw the PR.

@Marcono1234
Copy link
Collaborator

Apparently there is also another PR adding a Bazel build configuration: #1574
Though that PR looks more complex and is probably outdated by now.

@eamonnmcmanus
Copy link
Member

We recently removed the Gradle build because of the maintenance burden it imposed. So I think we'd be reluctant to add another build system. It happens that within Google we do build Gson with Blaze (the Google precursor to Bazel) but I'm still not really seeing the utility of having this. As @Marcono1234 notes, Bazel clients can get Gson from Maven Central.

Feel free to reopen this PR if there is some subtle reason why this is actually important for Bazel clients. In that case we would also need to update the build.yml workflow so that it validates the Bazel build as well as the Maven one.

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