Skip to content

Conversation

@nicoloboschi
Copy link
Contributor

@nicoloboschi nicoloboschi commented Oct 25, 2021

Motivation

In order to complete migration to Gradle we must build all the subprojects.

Changes

  • Enabled sh integration tests with gradle, located in tests/scripts/src/test/bash/gradle
  • Added these modules to the build
    • bookkeeper-http:servlet-http-server
    • metadata-drivers:etcd
    • tests:backward-compat:*
    • tests:shaded:*
    • stream:bk-grpc-name-resolver
  • DL shading process is now performed (before it didn't build any jar)
  • Groovy tests (tests:backward-compat:*) now are triggered by the build/tests itself; with Maven, there is a "runner" project (tests/integration-tests-base-groovy); in Gradle is useless so it is skipped

Test

  • Both bin/bookkeper standalone and bin/bookkeper_gradle standalone work locally
  • Tests are passing locally

Master Issue: #2849

@nicoloboschi nicoloboschi marked this pull request as draft October 25, 2021 15:57
@nicoloboschi nicoloboschi marked this pull request as ready for review October 26, 2021 10:21
@nicoloboschi nicoloboschi changed the title (WIP) Gradle build - some submodules are skipped Gradle build - some submodules are skipped Oct 26, 2021
@nicoloboschi
Copy link
Contributor Author

@eolivelli @pkumar-singh @hsaputra PTAL

echo ${BK_CLASSPATH}
else
add_maven_deps_to_classpath ${MODULE_PATH} >&2
cat ${BK_HOME}/${MODULE_PATH}/build/classpath.txt
Copy link
Member

Choose a reason for hiding this comment

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

This file is generated here. So If you need to change the name you have to change it there too.?
But I do not see any reason to change. Because we are not caching anything, with gradle. Gradle does the caching though.
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/build.gradle#L84

Copy link
Contributor Author

@nicoloboschi nicoloboschi Oct 27, 2021

Choose a reason for hiding this comment

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

I did the change because there was a sh test which checks the cached_classpath.txt file.
However I agree to duplicate sh tests in this case to facilitate maven removal, so I'm going to revert this change

@nicoloboschi
Copy link
Contributor Author

@pkumar-singh I created a new directory with the shell tests only for gradle. When we'll have to remove maven, we only have to drop the root directory and moves up the gradle one.

@nicoloboschi
Copy link
Contributor Author

rerun failure checks

@hsaputra
Copy link
Contributor

@nicoloboschi - I suppose you will update this PR from comments that @pkumar-singh have added?

Thanks

@hsaputra hsaputra self-requested a review October 27, 2021 21:15
@nicoloboschi
Copy link
Contributor Author

@hsaputra I already addressed Pkumar's comments. I updated the description of the PR. I think we are ready to merge it, if you like

Copy link
Contributor

@hsaputra hsaputra left a comment

Choose a reason for hiding this comment

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

LGTM +1

@hsaputra
Copy link
Contributor

Will merge this EOD PST if no objection or more comments. Thanks for the contribution @nicoloboschi !

@hsaputra
Copy link
Contributor

Will merge this one. We can iteratively add updates if anything a miss.

@hsaputra hsaputra merged commit 98ddf81 into apache:master Oct 29, 2021
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
### Motivation

In order to complete migration to Gradle we must build all the subprojects.

### Changes

- Enabled `sh` integration tests with gradle, located in `tests/scripts/src/test/bash/gradle`
- Added these modules to the build
    - `bookkeeper-http:servlet-http-server` 
    - `metadata-drivers:etcd`
    - `tests:backward-compat:*`
    - `tests:shaded:*`
    - `stream:bk-grpc-name-resolver`
- DL shading process is now performed (before it didn't build any jar)
- Groovy tests (`tests:backward-compat:*`) now are triggered by the build/tests itself; with Maven, there is a "runner" project (`tests/integration-tests-base-groovy`); in Gradle is useless so it is skipped


### Test

- Both `bin/bookkeper standalone` and `bin/bookkeper_gradle standalone` work locally
- Tests are passing locally  

Master Issue: apache#2849



Reviewers: Henry Saputra <hsaputra@apache.org>, Prashant Kumar <None>

This closes apache#2850 from nicoloboschi/fix/2849/gradle and squashes the following commits:

00b49f4 [Nicolò Boschi] Fix common_gradle.sh regex
bd739fd [Nicolò Boschi] fix sh tests
43230ba [Nicolò Boschi] revert sh files. Avoid to modify maven files, create gradle versions to faciltate migration
d1f95e4 [Nicolò Boschi] fix shaded deps
bcab40d [Nicolò Boschi] fix build
5fd0341 [Nicolò Boschi] fix build
0082e0e [Nicolò Boschi] fix build
2c32ac1 [Nicolò Boschi] fixes
3bc0b26 [Nicolò Boschi] bookkeeper-server-shaded-tests
ba89132 [Nicolò Boschi] shaded tests
6d39e33 [Nicolò Boschi] sh tests
e0032bc [Nicolò Boschi] actually run arquillian groovy tests
08dcc39 [Nicolò Boschi] backwards
2361f79 [Nicolò Boschi] hierarchical-ledger-manager
8388e11 [Nicolò Boschi] current-server-old-clients
6a24344 [Nicolò Boschi] bc-non-fips
2faca01 [Nicolò Boschi] bk-grpc-name-resolver
991bc11 [Nicolò Boschi] servlet-http-server
675ef7b [Nicolò Boschi] etcd
b1d5e14 [ZhangJian He] A empty implement in EtcdLedgerManagerFactory to let the project can compile (apache#2845)
bd5c50b [shustsud] Add error handling to readLedgerMetadata in over-replicated ledger GC (apache#2844)
746f9f6 [Matteo Merli] Remove direct ZK access for Auditor (apache#2842)
4117200 [ZhangJian He] the compare should be >= instead of > (apache#2782)
14ef56f [Prashant Kumar] BookieId can not be cast to BookieSocketAddress (apache#2843)
e10f3fe [ZhangJian He] Forget to close preAllocator log on shutdown (apache#2819)
53954ca [shustsud] Add ensemble check to over-replicated ledger GC (apache#2813)
919fdd3 [Prashant Kumar] Issue:2840 Create bookie shellscript for gradle (apache#2841)
031d168 [gaozhangmin] fix-npe-when-pulsar-ZkBookieRackAffinityMapping-getBookieAddressResolver (apache#2788)
3dd671c [Prashant Kumar] Migrate bookkeepr-server:test to gradle run unit tests excepts org.apache.bookkeeper.bookie. org.apache.bookkeeper.client org.apache.bookkeeper.replication org.apache.bookkeeper.tls. (apache#2812)
f6903b8 [Jack Vanlightly] BP-44 USE metrics
a4afaa4 [Matteo Merli] Eliminate direct ZK access in ScanAndCompareGarbageCollector (apache#2833)
a9b576d [Yunze Xu] Release semaphore when addEntry accepts the same entries (apache#2832)
148bf22 [Yun Tang] Ensure to release cache during KeyValueStorageRocksDB#closec (apache#2821)
4dc4260 [gaozhangmin] Heap memory leak problem when ledger replication failed (apache#2794)
a522fa3 [Raúl Gracia] Issue 2815: Upgrade to log4j2 to get rid of CVE-2019-17571 (apache#2816)
0465052 [Nicolò Boschi] Upgrade httpclient from 4.5.5 to 4.5.13 (apache#2793)
594a056 [Raúl Gracia] Issue 2795: Bookkeeper upgrade using Bookie ID may fail due to cookie mismatch (apache#2796)
354cf37 [Raúl Gracia] Upgraded dependencies with CVEs (apache#2792)
e413c70 [Raúl Gracia] Issue 2728: Entry Log GC may get blocked when using entryLogPerLedgerEnabled option (apache#2779)
883231e [pradeepbn] Building bookkeeper with gradle on java11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants