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

[WIP] Fixed netlayer and tor binary dependency issues #1914

Closed
wants to merge 8 commits into from

Conversation

freimair
Copy link
Contributor

netlayer had and has a well-hidden build configuration issue since version 0.4.2. It always fetches the "master" binaries instead of the binaries of the respective release. although until recently, bisq wanted to use 0.4.2, but actually has used 0.4.7 already. Due to changes in netlayer and its upcoming release, the netlayer build configuration has changed and thus, the issue showed itself.

This is an attempt to fix the issue temporarily. I, however, do expect failures due to checksum mismatches. Work in progress.

If this works, I will amend to the request and make things more final, i.e. create a proper release of netlayer and include that.

@freimair freimair requested a review from cbeams as a code owner November 13, 2018 09:37
@freimair
Copy link
Contributor Author

please do not merge this pull-request. I will supply a clean one if everything works again.

just using the travis CI here

@ManfredKarrer ManfredKarrer changed the title Fixed netlayer and tor binary dependency issues [WIP] Fixed netlayer and tor binary dependency issues Nov 13, 2018
@ManfredKarrer
Copy link
Contributor

Thanks for looking into it!

Copy link
Contributor

@cbeams cbeams left a comment

Choose a reason for hiding this comment

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

NACK per comments. Minor stuff, but the Maven Central change is more than just a nit and should actually be changed. Also, please review rules 3 and 5 from the commit comment guidelines at bisq-network/style#9 (no need to rebase/reword the commits in this PR, just for future reference). Thanks.

build.gradle Outdated
@@ -37,9 +37,9 @@ configure(subprojects) {
}

repositories {
mavenCentral()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't change this. There are issues with JCenter's SSL cert under OpenJDK 10, that's why I recently switched everything to Maven Central.

@@ -37,9 +37,9 @@ configure(subprojects) {
}

repositories {
mavenCentral()
maven { url 'https://jitpack.io' }
maven { url 'https://raw.githubusercontent.com/JesusMcCloud/tor-binary/master/release/' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't kill the whitespace after the opening brace.

@cbeams
Copy link
Contributor

cbeams commented Nov 13, 2018

Sorry, @freimair, I just noticed this is still WIP and that you said you'd clean it up once you know everything is working.

@ManfredKarrer
Copy link
Contributor

@freimair @cbeams Let me know when its ready to merge/ACKed by @cbeams (or @cbeams - merge yourself).

@freimair
Copy link
Contributor Author

working again. I am going to close this mess and open a clean 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.

3 participants