-
Notifications
You must be signed in to change notification settings - Fork 1.3k
build-logic: Migrate to Gradle 8.0 #7555
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
base: master
Are you sure you want to change the base?
build-logic: Migrate to Gradle 8.0 #7555
Conversation
The recommendation is to switch to the UniformRandomProvider. This would however add a new dependency that's only used in the unit tests. [1] https://issues.apache.org/jira/browse/LANG-1604
…tAsLong The buyerSecurityDepositAsLong field was superseded by the buyerSecurityDepositAsPercent field.
It's too risky to refactor the code because it's used in the account age verification logic.
WalkthroughThis PR adds widespread Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1)
75-82: Update the protobuf accessor to use the non-deprecated API instead of suppressing the warning.The
getExcludeFromJsonDataMap()call is deprecated in protobuf-java. Rather than suppressing the warning, migrate to the recommended accessor pattern: usegetExcludeFromJsonDataMap()(which returns an unmodifiable Map), or the helper methodsgetExcludeFromJsonDataOrDefault(key, default),getExcludeFromJsonDataOrThrow(key), etc., depending on the usage semantics. This pattern should be applied consistently across the 60+ similar files mentioned to avoid accumulating technical debt during the Gradle upgrade.desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java (1)
93-111: Fix unsafe array cast in SlidingWindowSpliterator.tryAdvance (runtime CCE risk)The current implementation:
action.accept(Arrays.stream((T[]) buffer.toArray(new Object[0])));casts an
Object[]toT[], which will throwClassCastExceptionat runtime for anyTother thanObject(e.g.,NumberinsimpleMovingAverage). The added@SuppressWarnings("unchecked")hides the compiler warning but not the actual bug.You can avoid both the warning and the runtime issue by snapshotting the buffer into a new collection and streaming from that:
- @SuppressWarnings("unchecked") @Override public boolean tryAdvance(Consumer<? super Stream<T>> action) { if (windowSize < 1) { return false; } while (sourceIterator.hasNext()) { buffer.add(sourceIterator.next()); if (buffer.size() == windowSize) { - action.accept(Arrays.stream((T[]) buffer.toArray(new Object[0]))); + // Snapshot current window to avoid unsafe array casts and later buffer mutations + action.accept(new ArrayDeque<>(buffer).stream()); buffer.poll(); return true; } } return false; }This keeps the original semantics (a snapshot of the current window) without any unchecked cast or runtime type issues, so the suppression can be removed.
🧹 Nitpick comments (16)
core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (1)
74-84: Consider adding a comment explaining the deprecation suppression.The annotation suppresses deprecation warnings from protobuf accessors during the Gradle 8.0 migration, but there's no inline comment explaining the rationale or indicating when this might be revisited (e.g., once protobuf APIs are updated).
Add a clarifying comment:
// Suppress deprecation warnings from protobuf accessors used during proto deserialization. // Can be removed once protobuf library updates these to non-deprecated methods (Gradle 8.0 migration). @SuppressWarnings({"deprecation", "unchecked"})This improves maintainability for future developers.
core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (1)
67-74: Method-level deprecation suppression here is reasonable and low-riskAdding
@SuppressWarnings("deprecation")directly onfromProtois a focused way to keep backward‑compatible deserialization while keeping the rest of the class warning‑clean. The constructor call and field mapping remain unchanged, so there’s no behavioral impact. Once the deprecated proto fields/types are finally removed, this suppression can be dropped as a follow‑up cleanup, but for now this looks good as part of the Gradle 8 migration.core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1)
70-77: Scoped deprecation suppression is fine; consider documenting the reason.The method-level
@SuppressWarnings("deprecation")is appropriately scoped and keeps build output clean without altering behavior. As an optional improvement, you might add a short comment indicating which deprecated proto field(s) this is tied to or reference the migration plan, so future cleanup is easier once those fields are removed.core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (1)
77-85: Scoped deprecation suppression on legacy proto mapping looks fineUsing
@SuppressWarnings("deprecation")on thefromProtofactory is an appropriate, tightly scoped way to deal with deprecated proto usage while keeping the public API intact for backward compatibility. If this deprecation is long‑lived across multiple payloads, consider adding a brief Javadoc or code comment somewhere central explaining which proto elements are deprecated and why they’re intentionally kept, to avoid future “cleanup” attempts removing necessary migration code.desktop/src/test/java/bisq/desktop/ComponentsDemo.java (1)
52-54: Method-level@SuppressWarnings("unchecked")is fine here; consider removing raw types laterGiven this is demo/test UI code and part of a Gradle 8 migration, a method-scoped
@SuppressWarnings("unchecked")onstartis reasonable and keeps the build clean. If you ever want to drop the suppression, you can tighten the generics usage (e.g.,ObservableList<String> list = FXCollections.observableArrayList();) so the compiler no longer emits unchecked warnings.build-logic/gradle-tasks/src/main/kotlin/bisq/gradle/tasks/PgpFingerprint.kt (2)
3-3: Prefer specific import over wildcard.Consider importing
java.util.Localeexplicitly instead of using a wildcard import for better clarity.-import java.util.* +import java.util.Locale
8-8: UseLocale.ROOTfor technical identifiers.PGP fingerprints are technical identifiers that should be normalized in a locale-independent manner. Using
Locale.getDefault()can introduce environment-specific behavior, which is inappropriate for cryptographic identifiers.- .lowercase(Locale.getDefault()) + .lowercase(Locale.ROOT)p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java (1)
67-85: Verify if this test provides value with assertions commented out.The
@SuppressWarnings("deprecation")annotation is appropriate for testing deprecated protobuf methods. However, the entire test body creates objects but performs no assertions (lines 79-84 are commented out with a TODO). This test may not provide meaningful coverage.Consider either:
- Implementing the assertions using the suggested resolvers, or
- Removing the test if it no longer serves a purpose
Would you like me to help determine if the test can be completed or should be removed?
core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java (1)
94-107: Add inline documentation explaining the protobuf deprecation suppression.This appears to be generated protobuf code with deprecated getter methods. The
@SuppressWarnings("deprecation")should include an inline comment documenting:
- Why the deprecated protobuf API is still used (e.g., which .proto field(s) are marked
deprecated = true)- Whether migration to non-deprecated alternatives is possible or planned
- Reference to any tracking ticket for eventual resolution
Per Java best practices, suppressing deprecations without documentation makes future maintenance difficult and should include at minimum a one-line justification pointing to the proto source or migration ticket.
desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java (1)
612-724: Centralizing deprecation suppression in the factory is reasonablePutting
@SuppressWarnings("deprecation")on this private switch-based factory keeps the many legacy payment methods usable without flooding the code with annotations. Consider tracking removal of truly obsolete methods separately so this suppression doesn’t hide future cleanup opportunities.desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java (1)
126-141: Raw/unchecked suppression is acceptable but could be narrowed with genericsThe added
@SuppressWarnings({"rawtypes", "unchecked"})keeps the warning local and doesn’t change behavior. Longer term, you could avoid the suppression by:
- Typing the
dataparameter more specifically (e.g.,@Nullable Tuple2<?, ?> datafor the BsqSendView case), or- Using
if (data instanceof Tuple2<?, ?> tuple) { ((BsqSendView) view).fillFromTradeData(tuple); }.Not required for this Gradle migration, but would eliminate the need for raw/unchecked suppression here.
desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java (1)
60-99: Deprecation suppression is fine short‑term; consider modernizing reflection usageExtending
@SuppressWarningshere to include"deprecation"is a reasonable way to keep builds clean given theconvention.newInstance()call below. Longer term, it would be safer to replaceClass#newInstance()withgetDeclaredConstructor().newInstance()and adjust the exception handling accordingly; that would remove the need for the deprecation suppression and align better with newer JDKs. Not blocking for this Gradle 8 migration, but worth tracking as a follow‑up.desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java (1)
242-243: Consider more targeted deprecation suppression.The
@SuppressWarnings("deprecation")annotation suppresses warnings for the entireaddContent()method (250+ lines). For better maintainability, consider:
- Identifying which specific API calls are deprecated
- Applying suppression more narrowly at the call site or in a smaller helper method
This makes it easier to track down and eventually address the deprecated usage when the API is removed.
core/src/main/java/bisq/core/offer/OpenOfferManager.java (1)
358-358: Verify migration runs only when needed.The migration method
maybeUpdatePersistedOffers()is invoked on every bootstrap. While the method has conditional checks to skip unnecessary updates, it still iterates through all open offers and performs multiple comparisons on each startup. Consider adding a migration version flag to skip this logic entirely once all offers have been migrated.gradle/verification-metadata.xml (2)
2-10: Schema bump OK; consider enabling signature verification next.Moving to dependency-verification-1.3.xsd looks good. Since verify-signatures is false, plan a follow-up to enable signature verification and manage trusted keys; it materially improves the supply‑chain posture.
2-4341: Prune unused coordinates to shrink the allowed artifact surface.Multiple versions of the same modules (Jackson, Guava, Gson, Kotlin, JaCoCo, JUnit) remain. After the Gradle 8 migration settles, re-run metadata generation with only resolved artifacts to reduce attack surface and review burden:
- Clear caches; run on clean CI.
gradle --write-verification-metadata sha256 --export-keyson a full build.I can draft a short “how we rotate verification metadata” doc/checklist if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (80)
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java(2 hunks)build-logic/gradle-tasks/src/main/kotlin/bisq/gradle/tasks/OS.kt(2 hunks)build-logic/gradle-tasks/src/main/kotlin/bisq/gradle/tasks/PgpFingerprint.kt(1 hunks)build-logic/packaging/src/main/kotlin/bisq/gradle/packaging/OS.kt(1 hunks)core/src/main/java/bisq/core/offer/OpenOfferManager.java(1 hunks)core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java(1 hunks)core/src/main/java/bisq/core/proto/CoreProtoResolver.java(2 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java(3 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java(1 hunks)core/src/main/java/bisq/core/user/PreferencesPayload.java(0 hunks)desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java(1 hunks)desktop/src/main/java/bisq/desktop/components/MenuItem.java(1 hunks)desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java(1 hunks)desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java(1 hunks)desktop/src/test/java/bisq/desktop/ComponentsDemo.java(1 hunks)gradle/verification-metadata.xml(38 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java(1 hunks)proto/src/main/proto/pb.proto(1 hunks)
💤 Files with no reviewable changes (1)
- core/src/main/java/bisq/core/user/PreferencesPayload.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Java 11, windows-latest
- GitHub Check: Test Java 11, macOS-latest
- GitHub Check: Test Java 11, ubuntu-latest
🔇 Additional comments (72)
core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (2)
74-84: Verify annotation coverage for both deprecation and unchecked warnings.The PR summary mentions suppressing both "deprecation and unchecked cast warnings," but Line 74 only suppresses
"deprecation". TheHashMapconstruction at Line 83 withproto.getExcludeFromJsonDataMap()typically triggers unchecked cast warnings in Java when converting from protobuf's generic Map type.Confirm whether the annotation should be expanded to:
@SuppressWarnings({"deprecation", "unchecked"})Alternatively, if unchecked warnings are handled elsewhere or intentionally left unsuppressed, clarify that distinction.
75-84: Implementation is correct and follows established pattern.The
fromProto()method correctly deserializes the protobuf object into the domain model. The logic, data extraction, and object construction are sound and align with the pattern used across other payment payload classes in this migration.core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (1)
89-106: Method-level deprecation suppression is reasonable hereLocalizing
@SuppressWarnings("deprecation")tofromProto(...)keeps the deprecation usage contained to this adapter layer and avoids polluting callers, which is appropriate given the broader migration. No issues from this change alone.core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (1)
94-109: Method‑level deprecation suppression looks appropriate hereScoping
@SuppressWarnings("deprecation")tofromProtokeeps the suppression local to the legacy deserialization path and avoids touching callers. This is consistent with the pattern used across other payloads in the PR and doesn’t alter behavior. LGTM as is.build-logic/packaging/src/main/kotlin/bisq/gradle/packaging/OS.kt (1)
10-10: LGTM! Kotlin API modernization.The change from
toLowerCase(Locale.US)tolowercase(Locale.US)correctly migrates to Kotlin's idiomatic API while preserving identical behavior.build-logic/gradle-tasks/src/main/kotlin/bisq/gradle/tasks/OS.kt (1)
10-10: LGTM! Kotlin API modernization.The change from
toLowerCase(Locale.US)tolowercase(Locale.US)correctly migrates to Kotlin's idiomatic API while preserving identical behavior.core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (1)
74-81: Scoped deprecation suppression onfromProtolooks goodAdding
@SuppressWarnings("deprecation")at the method level cleanly isolates suppression to this legacy proto-mapping code without changing behavior or hiding unrelated warnings. This is a reasonable compromise while the deprecated proto types are still in use.desktop/src/main/java/bisq/desktop/components/MenuItem.java (1)
145-146: LGTM!The
@SuppressWarnings("unchecked")annotation correctly suppresses the unavoidable warning from generic array creation. The variable-level scope is appropriately narrow.core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (1)
88-100: Clarify why protobuf API deprecation warnings require suppression and document the migration approach.The
@SuppressWarnings("deprecation")annotation suppresses warnings from protobuf APIs used in thefromProtomethod. Before accepting this approach as part of the Gradle 8.0 migration, clarify:
- Why these APIs are deprecated: Is this due to a protobuf version upgrade triggered by Gradle 8.0, or stricter compiler warnings?
- Whether alternatives exist: Are there non-deprecated replacement APIs, or are these the only available options?
- Long-term plan: Is this a temporary measure with a migration strategy, or permanent?
If deprecation suppression is necessary as a stopgap, add a comment in the code explaining why, the Gradle version constraint, and any planned follow-up work.
p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (1)
32-35: LGTM! Appropriate warning suppression for test mock.The localized
@SuppressWarnings("unchecked")annotation is a reasonable approach for suppressing generic type warnings in test mock constructors during the Gradle 8.0 migration.p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java (1)
47-51: LGTM! Standard suppression for Mockito usage in test mocks.The
@SuppressWarnings("unchecked")annotation appropriately handles generic type warnings from themock()calls. This is a standard pattern when using Mockito in test infrastructure and is properly scoped to the constructor.core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1)
67-74: Clarify which proto methods are deprecated and confirm the suppression strategy is documented.The
@SuppressWarnings("deprecation")on this method suppresses warnings without clarifying which specific proto getters are deprecated. Since this pattern appears across 60+ files in the PR:
- Specify which proto methods (
getPaymentMethodId(),getId(),getMaxTradePeriod(),getExcludeFromJsonDataMap()) are deprecated- Indicate whether the deprecation is in protobuf-generated code and if replacements exist
- Confirm this suppression approach is an accepted interim strategy for the migration or if targeted fixes are needed
core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (1)
80-88: Deprecation suppression onfromProtois appropriately scopedAdding
@SuppressWarnings("deprecation")at thefromProtomethod level is a reasonable way to contain noise from deprecated proto APIs while keeping call sites clean, and it aligns with the Gradle 8 migration’s build‑cleanliness goal. No further changes needed here.core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java (1)
86-96: Document what's deprecated and the migration timeline.The
@SuppressWarnings("deprecation")annotation suppresses warnings rather than addressing the underlying deprecation. As part of the Gradle 8.0 migration across multiple payment payload classes, clarify in code comments or the PR description: what API is deprecated (e.g.,protobuf.PaymentAccountPayload,getExcludeFromJsonDataMap(), or related getters), whether this is a temporary workaround, and if there's a planned migration path to non-deprecated alternatives.core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java (1)
77-77: Verify the deprecation suppression strategy and migration plan.The widespread addition of
@SuppressWarnings("deprecation")across 60+ proto deserialization methods suggests deprecated APIs in the proto layer. While this may be necessary for the Gradle 8.0 migration, suppressing deprecation warnings can mask technical debt.Please confirm:
- Which specific proto APIs are deprecated (e.g.,
getMaxTradePeriod(),getExcludeFromJsonDataMap(), or others)?- Is there a migration plan or timeline to address these deprecations?
- Should this technical debt be tracked with a TODO comment or linked issue?
core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (1)
89-106: Verify what protobuf API is deprecated and document any migration plan.The
@SuppressWarnings("deprecation")annotation hides compiler warnings without indicating which protobuf API or accessor is deprecated. While consistent with the project-wide pattern across 60+ payment payload classes for the Gradle 8.0 migration, clarify:
- Which protobuf API is deprecated (message type, method, or field)?
- Is there a documented migration path or timeline to address this?
If this is a temporary measure for the migration, consider adding a comment explaining the deprecation source and when it should be revisited.
core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (1)
82-94: The@SuppressWarnings("deprecation")suppression is appropriate here.The annotation targets protobuf-generated Map accessor methods (specifically
proto.getExcludeFromJsonDataMap()on line 93) that are deprecated in protobuf 3.x. This is a systematic pattern across 40+ payment payload classes and cannot be resolved by changing hand-written code, as the deprecated methods originate from protobuf code generation. The suppression is legitimate and necessary.core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java (1)
80-92: The@SuppressWarnings("deprecation")suppresses a temporary compatibility issue; document the protobuf version constraints and upgrade path.Protobuf v26.x removed long-marked deprecated Java APIs. The suppression is a pragmatic workaround for the Gradle 8 migration, but it creates technical debt. This approach is valid only if:
- The protobuf-java runtime version is pinned and won't be upgraded past the version that removed these APIs, OR
- There's a tracked plan to regenerate
.protosources or migrate to the non-deprecated API replacements when protobuf runtime is upgraded.Consider adding a comment explaining the version constraint or linking to a tracking issue for the proper migration (regenerate sources with matched protobuf-java and protoc versions, or update usages to non-deprecated replacements per protobuf migration notes).
core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (1)
89-100: Clarify the protobuf deprecation source and migration timeline.The
@SuppressWarnings("deprecation")addition is part of the Gradle 8.0 migration and reflects real deprecations in the protobuf ecosystem. Confirm whether the warnings originate from protobuf-generated message getters (likeproto.getPaymentMethodId(),proto.getId()) being marked deprecated in a newer protobuf-java version, or from the protobuf-gradle-plugin itself using deprecated Gradle APIs. If this is a known tactical measure, document the planned follow-up: either updating the protobuf-gradle-plugin to a Gradle 8-compatible version, regenerating protobuf sources with an updated protoc compiler, or explicitly migrating deprecated protobuf Java API calls to their replacements.core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (1)
81-82: Ensure deprecation suppressions follow migration best practices and include documentation.While
@SuppressWarnings("deprecation")is the correct annotation, best practice for large migrations requires:
Add an inline comment explaining the deprecation source, replacement API (if available), issue tracker reference, and planned removal date. Example:
// TODO: Remove after Gradle 8.0 migration complete (see ISSUE-XXX). // Deprecated by: [API/plugin], migrate to: [replacement] @SuppressWarnings("deprecation") public static CashByMailAccountPayload fromProto(...) {Verify this is part of an organized migration effort—check if upgrading the protobuf Gradle plugin to 0.9.x or later eliminates these warnings instead of suppressing them.
Ensure suppressions are tracked in your migration plan with a removal target date or milestone.
Without documentation linking suppressions to specific issues and removal dates, this pattern (repeated 60+ times) risks becoming permanent technical debt.
core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (1)
80-92: Remove @SuppressWarnings("deprecation") or document the specific deprecation being suppressed.The standard protobuf-java accessor methods (getPaymentMethodId(), getId(), etc.) are not deprecated. If this annotation suppresses a real warning, clarify which method is deprecated and why a non-deprecated alternative isn't used. Blanket suppression of deprecation warnings should be avoided unless necessary.
core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (1)
75-83: Verify whether proto-generated deprecation warnings should be suppressed at method level or build configuration.The
@SuppressWarnings("deprecation")annotation addresses deprecation warnings, but the approach and permanence should be clarified. Web search indicates that for Gradle 8.9 protobuf deprecation contexts, suppression is typically handled at theJavaCompiletask level rather than per-method. Verify:
- Whether these deprecation warnings originate from proto-generated code (which cannot be modified) or from actual API deprecation that should be addressed
- Whether a build-level suppression strategy (
JavaCompiletask configuration) is already in place across the project- If this method-level annotation is temporary during migration or the permanent solution
If suppression is the intended long-term approach, document the rationale and ensure consistency across the 60+ affected files.
core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (1)
73-80: Scoped deprecation suppression on fromProto looks goodLimiting the deprecation suppression to this factory method is appropriate and keeps the warning localized without changing behavior.
core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (1)
82-91: Deprecation suppression on fromProto is appropriately scopedThe added
@SuppressWarnings("deprecation")on this factory keeps the warning localized and does not affect the existing behavior or API.core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (1)
101-115: fromProto deprecation suppression aligns with other payloadsConsistent use of
@SuppressWarnings("deprecation")on this deserialization factory is fine and keeps the warning surface minimal.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java (1)
196-244: Targeted deprecation suppression on maybeRepublishTradeStatistics is fineThe annotation is confined to this method that bridges older trade/statistics types; behavior and logging remain unchanged.
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java (2)
81-87: Narrow deprecation suppression for TakeBuyBSQOfferTest is fineLocal
@SuppressWarnings("deprecation")on the deprecated test helper keeps the warning tight to its usage and doesn’t alter the test flow.
104-110: Same for TakeSellBSQOfferTest deprecation suppressionAgain, scoping the suppression to the single deprecated helper instantiation is a good balance between signal and noise in the test suite.
core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java (1)
73-80: Scoped deprecation suppression on fromProto looks goodAdding
@SuppressWarnings("deprecation")directly onfromProtokeeps the unavoidable proto deprecation noise localized without changing behavior. This is consistent with the pattern across other payloads.core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java (1)
82-90: Consistent deprecation suppression on Uphold fromProtoThe added
@SuppressWarnings("deprecation")onfromProtois consistent with other payloads and keeps proto compatibility code from spamming warnings, with no behavioral change.core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java (1)
83-92: SBP fromProto deprecation suppression is straightforwardAdding
@SuppressWarnings("deprecation")onfromProtocleanly scopes the proto deprecation warnings to this compatibility method and doesn’t alter behavior.core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (1)
95-104: Revolut fromProto deprecation suppression aligns with rest of payloadsThe new
@SuppressWarnings("deprecation")onfromProtois consistent with other account payloads and keeps proto deprecation noise localized without changing deserialization behavior.core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (1)
102-121: SpecificBanks fromProto deprecation suppression is appropriateGiven this method’s detailed mapping over nested proto payloads, placing
@SuppressWarnings("deprecation")onfromProtois a reasonable way to isolate proto deprecation warnings to this compatibility path, with no change in logic.core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (1)
75-86: MercadoPago fromProto deprecation suppression matches global patternThe
@SuppressWarnings("deprecation")onfromProtois in line with other payment payloads and focuses deprecation silencing on the proto construction path only, without affecting runtime behavior.core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (1)
74-84: Paytm fromProto deprecation suppression is safe and localizedThe added
@SuppressWarnings("deprecation")onfromProtokeeps the unavoidable proto deprecation warnings confined to this deserialization method and doesn’t alter its semantics.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java (1)
57-57: Appropriate use of deprecation suppression for migration code.The
@SuppressWarnings("deprecation")annotations on the constructor and conversion methods are reasonable here, as this class specifically handles backward-compatible migration from the deprecatedTradeStatistics2format toTradeStatistics3. This is a valid use case for suppression in legacy data conversion pathways.Also applies to: 119-119, 159-159
core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (1)
72-79: Consistent deprecation suppression pattern.This follows the same pattern as other payment payload classes in this PR, suppressing deprecation warnings during protobuf deserialization. The approach is consistent across the codebase.
core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (1)
75-84: Symmetric suppression for serialization paths.The deprecation suppressions on both
toProtoMessage()andfromProto()maintain symmetry in the serialization/deserialization logic. This is consistent with the broader PR pattern and ensures backward compatibility.Also applies to: 86-95
core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (1)
71-78: LGTM - Consistent with payment payload pattern.core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1)
74-84: LGTM - Consistent with payment payload pattern.proto/src/main/proto/pb.proto (1)
1923-1923: Verify removal of buyer_security_deposit_as_long field.The deprecated field
buyer_security_deposit_as_longhas been removed and tag 30 properly reserved. Before merging, manually verify that:
- No code attempts to read or write this field (search for
buyer_security_deposit_as_long,getBuyerSecurityDepositAsLong, andsetBuyerSecurityDepositAsLong)- PreferencesPayload deserialization handles the missing field gracefully for backward compatibility
- Any functionality that relied on this field has been migrated to an alternative
core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (1)
106-115: Remove or clarify the @SuppressWarnings("deprecation") – protobuf newBuilder() is not deprecated.The suppression assumes
protobuf.PaymentAccountPayload.newBuilder()is deprecated, but Protocol Buffers Java does not marknewBuilder()as deprecated as of 2024–2025. The protobuf Javadoc and release notes continue to reference it as a standard API. If a deprecation warning is present, it likely originates from internal Bisq code rather than the protobuf library itself. Either remove the suppression if no warning is generated, or clarify and target the actual source of the deprecation.Likely an incorrect or invalid review comment.
core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (1)
72-79: @SuppressWarnings("deprecation") is already applied—verify which protobuf API is deprecated.The
fromProtomethod already includes@SuppressWarnings("deprecation")to suppress warnings in this and other payment payload classes as part of the Gradle 8.0 migration. This suppression appears consistent across the payment payload implementations.To ensure this is the appropriate solution:
- Identify which specific protobuf API or method is marked deprecated (likely a generated message method in the proto classes).
- Verify that no simple refactoring can replace the deprecated call rather than suppress it indefinitely.
- Document or track the underlying deprecation for future remediation when the protobuf API evolves.
Without access to the protobuf compiler output, build logs, or the
@Deprecatedannotation in the generated proto classes, the root cause cannot be confirmed externally. If this suppression appears across 60+ files as stated, consider whether a blanket suppression at the package level (package-info.java) would be more maintainable than per-method annotations.core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (1)
142-165: LGTM!The
@SuppressWarnings("deprecation")annotation is appropriately scoped to the method level and is consistent with the broader pattern across payment payload classes in this PR. This is a reasonable approach for handling deprecated protobuf accessor methods during the Gradle 8.0 migration.core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (1)
73-80: LGTM!Consistent with the PR-wide pattern of adding method-level deprecation suppression to
fromProtomethods.core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (1)
67-74: LGTM!Follows the established pattern for deprecation suppression on proto deserialization methods.
core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (1)
74-84: LGTM!Consistent application of method-level deprecation suppression for proto deserialization.
core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (1)
80-92: LGTM!The
@SuppressWarnings("deprecation")annotation follows the consistent pattern applied across all payment payload classes in this PR.core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java (1)
77-85: LGTM - Part of systematic Gradle 8.0 migration.The deprecation suppression on
fromProtois consistent with the broader migration pattern across payment payload classes.desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java (1)
200-237: LGTM - Deprecation suppression for offer cloning.The suppression is appropriately scoped to the local variable using the deprecated
OfferPayloadconstructor, which remains necessary for the cloning functionality.core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (1)
70-77: LGTM - Consistent with migration pattern.Deprecation suppression aligns with the systematic approach across payment payload deserialization methods.
core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (2)
97-113: LGTM - Proto serialization suppression.The deprecation suppression on
toProtoMessagecomplements thefromProtosuppression below, indicating deprecated proto schema elements in both serialization paths.
115-129: LGTM - Proto deserialization suppression.Consistent with the broader migration pattern for payment payload classes.
core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (1)
72-80: LGTM - Standard migration pattern.Deprecation suppression consistent with other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (1)
94-110: LGTM - Consistent suppression pattern.The deprecation suppression follows the same pattern applied across payment payload deserialization methods.
core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java (1)
110-123: LGTM - Proto deserialization suppression.Consistent with the migration pattern for payment payload classes.
core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2)
231-282: LGTM - Offer payload serialization suppression.The deprecation suppression on
toProtoMessageis part of the systematic migration, likely related to the removal of the deprecatedbuyer_security_deposit_as_longproto field mentioned in the PR summary.
284-337: LGTM - Offer payload deserialization suppression.Consistent with the
toProtoMessagesuppression above, completing the bidirectional proto serialization handling.core/src/main/java/bisq/core/offer/OpenOfferManager.java (4)
959-974: Migration logic correctly skips BsqSwap offers.The method appropriately skips BsqSwap offers since they don't have fee transactions and can be recreated without cost. The ConcurrentModificationException prevention via cloning is also correct.
975-997: Capability migration logic is sound.The method correctly checks for missing MEDIATION and REFUND_AGENT capabilities, preserves existing extra data, and updates the capabilities string. The conditional logic properly handles both cases where capabilities need updating and where they don't.
1053-1074: State preservation and replacement logic is correct.The migration properly:
- Preserves original states before modification
- Marks old offer as REMOVED/CANCELED
- Creates new offer with updated payload
- Restores preserved states to new offer
- Maintains list consistency
This ensures data integrity during the migration.
1014-1051: Verify all OfferPayload fields are copied.The OfferPayload reconstruction copies multiple fields from the original, which is essential to prevent data loss during migration. However, without comparing the constructor parameters against all fields defined in the OfferPayload class, it's unclear if all fields are included. If new fields are added to OfferPayload in the future, this manual copying approach could silently fail to include them.
core/src/main/java/bisq/core/proto/CoreProtoResolver.java (2)
101-259: Verify deprecation warnings are expected.The
@SuppressWarnings("deprecation")annotation onfromProto(protobuf.PaymentAccountPayload)suppresses warnings from the proto deserialization methods. Given the PR's goal of migrating to Gradle 8.0, ensure that:
- These deprecations are documented in the proto-generated code or related classes
- No non-deprecated alternative exists for proto deserialization
- The suppression is intentional and not a workaround that could be avoided through other means
261-285: Verify deprecation warnings are expected.The
@SuppressWarnings("deprecation")annotation onfromProto(protobuf.PersistableNetworkPayload)suppresses warnings from calling deprecatedfromProtomethods on TradeStatistics2, TradeStatistics3, and other payload classes. Ensure these deprecations are documented with@Deprecatedannotations and javadoc explaining the reason for deprecation and any recommended alternatives, confirming backward compatibility is the intended justification.gradle/verification-metadata.xml (8)
85-95: Manual SHA entries — please re-generate from a clean environment or attach provenance.Entries with origin="Manually computed by Alva" are fine temporarily but increase risk of mismatch. Prefer re-writing via Gradle on a clean machine (no cache) and storing provenance (source URL, sig/asc).
Also applies to: 931-947, 1293-1299, 2552-2614, 4046-4054, 4063-4071, 4079-4085
3034-3100: JaCoCo upgraded to 0.8.11 — verify if 0.8.8 entries are unused before removing.The snippet shows both 0.8.11 and 0.8.8 versions present. Before pruning the 0.8.8 entries to reduce allowed artifacts, confirm that the build no longer requires or resolves them.
3169-3761: Kotlin version consolidation: Remove legacy 1.3/1.6/1.7 artifacts if they are no longer used.The verification-metadata.xml contains entries for Kotlin 1.3.41, 1.6.20, 1.7.0, 1.7.10, and 1.9.23. If the build has fully migrated to 1.9.23, the older entries should be removed to reduce metadata maintenance and avoid accidental resolution of obsolete dependencies. This requires confirming that no active build configuration references versions 1.3.x, 1.6.x, or 1.7.x.
972-987: Develocity plugin hashes added — verify plugin configuration and consider upgrading version.The migration to com.gradle.develocity 3.17.5 is compatible with Gradle 8.x. Ensure buildSrc/settings.gradle explicitly applies
id("com.gradle.develocity") version "3.17.5"and remove the old gradle-enterprise-gradle-plugin coordinates if no longer referenced elsewhere. Consider upgrading to Develocity 3.19+ or 4.x for optimal Gradle 8.x and future compatibility.
2990-3002: No JDK compatibility restrictions from foojay-resolver 0.7.0 itself.This version requires Java 8+ to run and Gradle 7.6+ (compatible with Gradle 8.x), but does not limit which JDK versions can be resolved — it proxies Foojay's Disco API and resolves whatever distributions are available there. Verify Foojay/Disco API coverage instead if you need specific JDK versions (e.g., 17/21).
Likely an incorrect or invalid review comment.
4132-4160: ASM 9.6 entries added — LGTM.ASM 9.6 adds support for Java 22 bytecode and is fully compatible with Kotlin 1.9.x (which targets up to Java 21) and Gradle 8.x. This aligns well with modern Kotlin/Gradle toolchains.
Also applies to: 4177-4181, 4198-4205, 4222-4229
931-947: Verify Windows .exe and binary hashes against official upstream releases.The Windows protoc-3.19.1 hash in the file does not match official GitHub release checksums (official win64 SHA256: 7E904214696702285AA5B503A86FA38A14C9EEB0F3FE10FF3115A74E3A90DAE8). For protoc-gen-grpc-java 1.42.1, official SHA256 values are available on Maven Central. Confirm all hashes match upstream before commit, and consider removing unused platform artifacts to reduce maintenance burden.
2972-2989: Kotlin DSL plugin 4.4.0 — verify compatibility with the Gradle version used in this PR.gradle-kotlin-dsl-plugins 4.4.0 (released April 2024) is designed for Gradle 8.x and generally works with Gradle 8.0 through 8.9. However, Gradle's best practice is to use a kotlin-dsl plugin version that matches your specific Gradle release. Confirm which Gradle version this project targets and ensure 4.4.0 aligns with it.
Also applies to: 2985-2989
| public void copyInputStreamToFileNew(final InputStream source, | ||
| final File destination, | ||
| int fileSize) throws IOException { | ||
| try (final FileOutputStream output = FileUtils.openOutputStream(destination)) { | ||
| final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; | ||
| long count = 0; | ||
| int n; | ||
| while (EOF != (n = source.read(buffer))) { | ||
| output.write(buffer, 0, n); | ||
| count += n; | ||
| log.trace("Progress: {}/{}", count, fileSize); | ||
| updateProgress(count, fileSize); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputStream is no longer closed → add it to try‑with‑resources
The refactor closes only the FileOutputStream; the InputStream source is never closed and cannot be closed by the caller (created inline via urlConnection.getInputStream()), leading to a resource leak.
You can keep the modern style and ensure both resources are closed by including source in the try‑with‑resources header:
- public void copyInputStreamToFileNew(final InputStream source,
- final File destination,
- int fileSize) throws IOException {
- try (final FileOutputStream output = FileUtils.openOutputStream(destination)) {
- final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
- long count = 0;
- int n;
- while (EOF != (n = source.read(buffer))) {
- output.write(buffer, 0, n);
- count += n;
- log.trace("Progress: {}/{}", count, fileSize);
- updateProgress(count, fileSize);
- }
- }
- }
+ public void copyInputStreamToFileNew(final InputStream source,
+ final File destination,
+ int fileSize) throws IOException {
+ try (InputStream in = source;
+ FileOutputStream output = FileUtils.openOutputStream(destination)) {
+ final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
+ long count = 0;
+ int n;
+ while (EOF != (n = in.read(buffer))) {
+ output.write(buffer, 0, n);
+ count += n;
+ log.trace("Progress: {}/{}", count, fileSize);
+ updateProgress(count, fileSize);
+ }
+ }
+ }This restores deterministic cleanup for the HTTP connection while staying compatible with Gradle 8 / modern Java patterns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void copyInputStreamToFileNew(final InputStream source, | |
| final File destination, | |
| int fileSize) throws IOException { | |
| try (final FileOutputStream output = FileUtils.openOutputStream(destination)) { | |
| final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; | |
| long count = 0; | |
| int n; | |
| while (EOF != (n = source.read(buffer))) { | |
| output.write(buffer, 0, n); | |
| count += n; | |
| log.trace("Progress: {}/{}", count, fileSize); | |
| updateProgress(count, fileSize); | |
| } | |
| public void copyInputStreamToFileNew(final InputStream source, | |
| final File destination, | |
| int fileSize) throws IOException { | |
| try (InputStream in = source; | |
| FileOutputStream output = FileUtils.openOutputStream(destination)) { | |
| final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; | |
| long count = 0; | |
| int n; | |
| while (EOF != (n = in.read(buffer))) { | |
| output.write(buffer, 0, n); | |
| count += n; | |
| log.trace("Progress: {}/{}", count, fileSize); | |
| updateProgress(count, fileSize); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java
around lines 127 to 139, the InputStream parameter `source` is not closed and
causes a resource leak; include the passed `source` in the try-with-resources
header so both the InputStream and the FileOutputStream are closed
deterministically (e.g. try (InputStream s = source; FileOutputStream output =
...)), remove any manual close attempts, and keep the rest of the loop logic
unchanged.
| distributionBase=GRADLE_USER_HOME | ||
| distributionPath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.3-bin.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Gradle 8.x compatibility changes in the codebase
# Check if Java toolchain is configured in build files (required for Gradle 8.x)
echo "=== Checking for Java toolchain configuration ==="
rg -t gradle -t kotlin 'java\s*\{\s*toolchain' gradle/ 2>/dev/null || echo "No matches found"
# Check for gradle-wrapper.properties configuration
echo -e "\n=== Wrapper configuration ==="
grep -E 'distributionUrl|networkTimeout' gradle/wrapper/gradle-wrapper.properties 2>/dev/null || echo "File not found or no matches"
# Look for deprecated Upload task usage (removed in Gradle 9.0)
echo -e "\n=== Checking for deprecated Upload task ==="
rg 'Upload\s*\{' -t gradle -t kotlin 2>/dev/null || echo "No matches found"
# Check for plugin updates or version configurations in build.gradle files
echo -e "\n=== Checking main build configuration ==="
fd -e 'gradle.kts' -o -e 'gradle' | head -10
# Check for @SuppressWarnings deprecation annotations
echo -e "\n=== Checking for @SuppressWarnings(\"deprecation\") ==="
rg '@SuppressWarnings\s*\(\s*"deprecation"\s*\)' --max-count 20Repository: bisq-network/bisq
Length of output: 178
🌐 Web query:
Gradle 8.9 release stability and breaking changes from Gradle 7.6
💡 Result:
Short answer: Gradle 8.9 is a stable 8.x patch release (not an experimental preview) that continues the 8.x migration path from 7.6 — it adds improved error/warning and IDE reporting, bugfixes and small usability changes while inheriting the 8.0+ breaking/behavior changes you must address when moving from 7.6. Key upgrade points (from 7.6 → 8.9):
What to expect (stability)
- Gradle 8.9 is a normal stable Gradle 8.x release intended for production use; follow the wrapper upgrade (./gradlew wrapper --gradle-version=8.9). [1]
Most important breaking / migration items coming from 7.6
- Toolchain spec strictness / invalid specs: Gradle 7.6 began deprecating “invalid” toolchain specs (criteria without language-version); those deprecated usages become errors in 8.0+. Fix toolchain declarations to include language version. [2]
- Kotlin / Kotlin DSL changes: Kotlin versions embedded/upgraded across 8.x (and strict accessor generation for Kotlin DSL precompiled script plugins became the default in 8.0). Update any Kotlin-dsl script/plugin code and precompiled accessor usage. [2][3]
- Java toolchain provisioning marker change (8.9): toolchain provisioning behavior/marker file changed in 8.9 — existing toolchains will be re-provisioned first time used with 8.9 and an 8.9-provisioned toolchain won’t be re-provisioned if you downgrade. Plan CI/agent caching accordingly. [3]
- JavaCompile/JRE enforcement (8.x): some JavaCompile tasks may fail if only a JRE (no compiler) is present because toolchain resolution enforces a compiler; set source/target/release or provide a JDK toolchain. [3]
- Library/embedded dependency upgrades: various native library upgrades across 8.x (Kotlin, Apache SSHD/JSch replacement, JGit, Commons Compress) can change runtime behavior or artifact checksums (e.g., archive metadata). Test artifact-consuming pipelines. [3]
- Deprecated Gradle API removals: several formerly-deprecated util APIs were marked for removal in later majors — check deprecation warnings (run with --warning-mode=all or a Build Scan) and update plugin code. [2][3]
8.9-specific notes (small/usable changes)
- Improved dependency-variant error messages, better Java compilation reporting for IDEs, more details about selected JVMs, safer build init (won’t overwrite files without confirmation), TestNG suite thread control, etc. No large new breaking behavior beyond the 8.x migration items above. [1]
Recommended upgrade steps
- Run your build with Gradle 7.6 using gradle help --warning-mode=all (or help --scan) and fix deprecations. [2]
- Update plugins to compatible versions.
- Upgrade the wrapper to 8.9 and run full CI/tests; pay special attention to toolchains, Kotlin DSL precompiled accessors, and any archive/SSH/JGit-related behavior. [1][3]
Sources
- Gradle 8.9 release notes. [1]
- Gradle upgrade notes: “Upgrading from 7.x to 8.0” (deprecations/behavior changes starting 8.0). [2]
- Gradle “Upgrading within 8.x” / 8.9 migration notes (toolchain provisioning, JavaCompile behavior, Kotlin upgrades). [3]
If you want, I can list exact build.gradle/.kts changes to fix the common breakages (toolchain examples, Kotlin-dsl accessor switches, or how to ensure a JDK is used).
Gradle 8.9 wrapper update is syntactically correct; verify mandatory code-level changes before merge.
The URL format and version choice are valid. However, upgrading from Gradle 7.6 to 8.9 requires code-level fixes beyond deprecation suppressions:
Mandatory fixes required:
- Toolchain specs: Gradle 8.0+ enforces
language-versionin toolchain declarations (deprecated in 7.6, now errors). Addlanguage-version = JavaLanguageVersion.of("XX")to all toolchain blocks. - Java compiler enforcement: Some
JavaCompiletasks will fail if only a JRE is present. Ensure toolchain configuration provides a JDK or explicitly setsource/target/release. - Kotlin DSL: Strict accessor generation is now default in 8.0+. Verify precompiled script plugins and Kotlin DSL accessors are updated.
Verify before merge:
- Run
gradle help --warning-mode=allagainst the full codebase to identify remaining deprecation warnings. - Confirm all toolchain configurations include explicit language versions.
- Test that
JavaCompiletasks resolve to a JDK, not just a JRE.
The 60+ @SuppressWarnings("deprecation") annotations indicate incomplete migration; suppressing warnings alone will not prevent build failures from enforced breaking changes in Gradle 8.9.
🤖 Prompt for AI Agents
In gradle/wrapper/gradle-wrapper.properties around line 3 (distributionUrl
updated to Gradle 8.9), you must update project build files to be compatible
with Gradle 8.0+ by: finding every java toolchain block and adding an explicit
language-version (e.g. language-version = JavaLanguageVersion.of("17") or the
project target), ensuring JavaCompile tasks resolve a JDK (configure toolchains
to provide a JDK or set source/target/release explicitly where appropriate),
updating Kotlin DSL/precompiled script plugins and accessor generation to the
stricter Gradle 8 defaults, removing reliance on
@SuppressWarnings("deprecation") as a fix, then run gradle help
--warning-mode=all and full builds/tests to iterate on and fix any remaining
deprecations or breaking changes before merging.
Summary by CodeRabbit
Chores
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.