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

Create platform-dependent artifact #14

Merged
merged 3 commits into from
Jan 14, 2021

Conversation

erip
Copy link
Contributor

@erip erip commented Jan 11, 2021

Consumers of sentencepiece-jni need to depend on an artifact that is compatible with their desired platform. This PR seeks to add a discriminating classifier to the build which allows a user to specify which platform they're targetting a la os-maven-plugin. This has implications on current users as they will need to explicitly add the platform they were targeting before a cross-platform build was enabled (i.e., linux-x86_64).

@erip
Copy link
Contributor Author

erip commented Jan 11, 2021

See here for an example of the new artifact name with discriminating information for consumers.

@erip erip changed the title Create platform-dependent artifact [WIP] Create platform-dependent artifact - DONOTMERGE Jan 11, 2021
@erip erip changed the title [WIP] Create platform-dependent artifact - DONOTMERGE [WIP] Create platform-dependent artifact - DO NOT MERGE Jan 11, 2021
@erip erip force-pushed the feature/platform-dependent-artifact branch from 5d7bb82 to 3cc0591 Compare January 12, 2021 17:38
@erip erip changed the title [WIP] Create platform-dependent artifact - DO NOT MERGE Create platform-dependent artifact Jan 12, 2021
@erip
Copy link
Contributor Author

erip commented Jan 12, 2021

I think this is OK now @levyfan. Do you have any thoughts? The way to add this project as dependency now is, for example:

  <dependencies>
    <dependency>
      <groupId>com.github.levyfan</groupId>
      <artifactId>sentencepiece</artifactId>
      <version>0.0.1-SNAPSHOT</version>
      <classifier>osx-x86_64</classifier>
    </dependency>
  </dependencies>

which can be automated with the os-maven-plugin transparently assuming the plugin is included in the POM:

  <dependencies>
    <dependency>
      <groupId>com.github.levyfan</groupId>
      <artifactId>sentencepiece</artifactId>
      <version>0.0.1-SNAPSHOT</version>
      <classifier>${os.detected.classifier}</classifier>
    </dependency>
  </dependencies>

erip pushed a commit to mitre/jfairseq that referenced this pull request Jan 13, 2021
erip pushed a commit to mitre/jfairseq that referenced this pull request Jan 13, 2021
* add CI.

* use checkout action to clone repo.

* migrate to SBT build because gradle cannot handle classifiers.

* remove settings gradle file.

* update README to reflect sbt refactor.

* fix issue where published JAR only contains javadocs.

* add erip fork of sentencepiece while waiting for levyfan/sentencepiece-jni#14

* fix CI to use different JDKs in artifact build.

* update java opts so CI does not run out of memory.

* update temporarily required dep.
@levyfan levyfan merged commit 8666eaa into levyfan:master Jan 14, 2021
@levyfan
Copy link
Owner

levyfan commented Jan 14, 2021

I suppose we could further merge pom.xml and pom-windows.xml together by maven profile https://stackoverflow.com/questions/38465711/choose-maven-profile-from-os-family

@erip erip deleted the feature/platform-dependent-artifact branch January 14, 2021 12:00
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

2 participants