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

Add developer documentation #678

Merged
merged 5 commits into from
Mar 6, 2025
Merged

Conversation

DSouzaM
Copy link
Member

@DSouzaM DSouzaM commented Jan 15, 2025

While debugging a NI agent issue I found myself needing to modify and debug the Gradle plugin. This change adds some developer documentation to explain setup and some common workflows (at least, the ones I needed).

Tracked internally as GR-61396.

I also tried to incorporate the suggestions in #656 w.r.t. environment setup.

Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Jan 15, 2025
@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Jan 15, 2025
```shell
./gradlew publishAllPublicationsToCommonRepository
```bash
./gradlew :native-maven-plugin:publishToMavenLocal --no-parallel
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tried this command if it works?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it will work. Although personally I'd avoid polluting maven local and stick with the "common repository" (I usually add a local repository to Gradle/Maven builds to avoid the erratic behavior of maven local).

Copy link
Member Author

@DSouzaM DSouzaM Mar 3, 2025

Choose a reason for hiding this comment

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

@melix could you explain how to use the common repository? After grepping this repo I still don't really understand how it works.

I tried ./gradlew publishAllPublicationsToCommonRepository --no-parallel but my changes were not picked up
(presumably I need to use it in my project's build definition).

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is nothing special here, publishAllPublicationsToCommonRepository will update the repository found in build/common-repo, and for sure you need to use it in your project definition. For example in Gradle you'd simply add a repository which points to build/common-repo, and in Maven you'd add a <repository>. You don't have to go that way, you can publish to Maven local, but I personally think it's a bad practice, because Maven local is polluted with dozens of libraries which may make us accidentally think that a build is working, when if you try it on a different machine, it would break.


```bash
./gradlew publishToMavenLocal --no-parallel
./gradlew :native-gradle-plugin:publishToMavenLocal --no-parallel
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tried this command if it works?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it works. It publishes only the gradle plugin and not the maven one.

# build.gradle
plugins {
...
- id 'org.graalvm.buildtools.native' version '0.9.25'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need both this and SNAPSHOT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote this as a "diff" code sample to show how the existing build.gradle file might change (in this case, replacing a real release with a snapshot release)

DEVELOPING.md Outdated
For Maven, simply bump the version and it should try the local repository automatically:
```bash
# pom.xml
- <native.maven.plugin.version>0.9.25</native.maven.plugin.version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, why do we need this and SNAPSHOT?

DEVELOPING.md Outdated
The native-build-tools repo is set up as a multi-project Gradle project, with the Maven and Gradle plugins declared as subprojects of the root project.
To set the project up in your IDE (e.g., IntelliJ IDEA), import the root project and the IDE should automatically import the subprojects.

## Building & testing
Copy link
Collaborator

Choose a reason for hiding this comment

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

@melix can you please verify this section, because I didn't use these commands in the past?

@dnestoro
Copy link
Collaborator

Hey @DSouzaM, thanks for this PR. It will be really helpful. I just left some minor suggestions/questions, but in general, the PR looks good!

@dnestoro dnestoro requested review from olyagpl and melix February 13, 2025 10:11
olyagpl
olyagpl previously approved these changes Feb 13, 2025
@DSouzaM
Copy link
Member Author

DSouzaM commented Mar 3, 2025

Sorry about the delay on my end! I expected a GitHub notification when there were comments/updates on the PR, but I guess not.

@DSouzaM
Copy link
Member Author

DSouzaM commented Mar 4, 2025

ok @melix, this is ready for review.

Copy link
Collaborator

@melix melix left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, left a few minor changes

./gradlew :native-gradle-plugin:assemble

# Build and run all tests
./gradlew build
Copy link
Collaborator

Choose a reason for hiding this comment

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

See you in 10 days :)

Copy link
Member Author

Choose a reason for hiding this comment

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

what's in 10 days? 😱

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean if you run all tests, all functional tests, all verification in a single build, on a single machine, you can grab a few cups of coffee :)

Copy link
Member Author

Choose a reason for hiding this comment

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

😆 good call, we should be conscientious of the planet (and our users' time)

DEVELOPING.md Outdated
The above command publishes to a "common" repository located at `build/common-repo`.

Next, update the project build files:
1. Update the version string. The version can be found manually by searching for the published artifacts in `~/.m2/repository/org/graalvm/buildtools/native/`, or alternatively by checking the `nativeBuildTools` property [here](gradle/libs.versions.toml).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Update the version string. The version can be found manually by searching for the published artifacts in `~/.m2/repository/org/graalvm/buildtools/native/`, or alternatively by checking the `nativeBuildTools` property [here](gradle/libs.versions.toml).
1. Update the version string. The version can be found manually by searching for the published artifacts in `build/common-repo`, or alternatively by checking the `nativeBuildTools` property [here](gradle/libs.versions.toml).

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, good catch

DEVELOPING.md Outdated
Comment on lines 22 to 29
# Build all projects
./gradlew assemble

# Build only the native-gradle-plugin (for example)
./gradlew :native-gradle-plugin:assemble

# Build and run all tests
./gradlew build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Build all projects
./gradlew assemble
# Build only the native-gradle-plugin (for example)
./gradlew :native-gradle-plugin:assemble
# Build and run all tests
./gradlew build
# Compile all projects
./gradlew assemble
# Run unit tests in all projects
./gradlew test
# Run functional tests in all projects
./gradlew funTest
# Compile only the native-gradle-plugin (for example)
./gradlew :native-gradle-plugin:assemble
# Build and run all tests, complete (and very long) build
./gradlew build

melix
melix previously approved these changes Mar 4, 2025
olyagpl
olyagpl previously approved these changes Mar 5, 2025
```

A snapshot will be published to `build/common-repo`. For more details, see the [Developer documentation](../DEVELOPING.md).
Copy link
Member

Choose a reason for hiding this comment

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

will be published or is published after you run the build command above?

DEVELOPING.md Outdated
./gradlew build
```


Copy link
Member

Choose a reason for hiding this comment

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

Two lines empty space

@dnestoro
Copy link
Collaborator

dnestoro commented Mar 5, 2025

@DSouzaM please resolve remaining comments, so we can merge this PR today

@DSouzaM DSouzaM dismissed stale reviews from olyagpl and melix via c026664 March 5, 2025 14:12
@dnestoro dnestoro merged commit d65f4a0 into graalvm:master Mar 6, 2025
137 checks passed
@DSouzaM DSouzaM deleted the md/improve-docs branch March 6, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants