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

Build supervisor with xgo #2318

Closed
wants to merge 2 commits into from
Closed

Build supervisor with xgo #2318

wants to merge 2 commits into from

Conversation

zolia
Copy link
Contributor

@zolia zolia commented Jun 1, 2020

should fix #2314
Packaging on MacOS is running fine, thats why we missed probably.
We prob need to cross compile anyways.

@zolia zolia force-pushed the build-supervisor-with-xgo branch 6 times, most recently from 81ab26e to b12d52f Compare June 1, 2020 14:25
@Waldz Waldz force-pushed the build-supervisor-with-xgo branch from 17fa03b to 67f36b6 Compare June 2, 2020 05:53
@tadaskay
Copy link
Contributor

tadaskay commented Jun 2, 2020

We are already crosscompiling and we don't need xgo for the supervisor. The proper way would be to use a separate go.mod for that sub-project instead of trying to work around go-ethereum quirks (which supervisor does not need at all).

@Waldz Waldz force-pushed the build-supervisor-with-xgo branch 2 times, most recently from ee31fbf to 2dac05d Compare June 2, 2020 06:22
@vkuznecovas
Copy link
Contributor

I'm with Tadas on this one. If we use XGO it's going to prolong our build times by quite a lot.

@Waldz Waldz force-pushed the build-supervisor-with-xgo branch from 2dac05d to 624e523 Compare June 2, 2020 06:54
@Waldz Waldz force-pushed the build-supervisor-with-xgo branch from 624e523 to a796c86 Compare June 2, 2020 06:54
@zolia
Copy link
Contributor Author

zolia commented Jun 2, 2020

We are using xgo for building node atm anyways. I agree that we should migrate to cgo, but there is problem with go-ethereum - again. I suggest to follow this PR to be approved: ethereum/go-ethereum#21041 and bump etherium dependency then.

For now I suggest to merge as it is.

Copy link
Contributor

@tadaskay tadaskay left a comment

Choose a reason for hiding this comment

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

I'll try to fix it, please don't merge

@Waldz Waldz force-pushed the build-supervisor-with-xgo branch 7 times, most recently from 0bcaab3 to 8360edc Compare June 2, 2020 09:31
@Waldz Waldz force-pushed the build-supervisor-with-xgo branch from 8360edc to f299853 Compare June 2, 2020 09:47
@Waldz Waldz force-pushed the build-supervisor-with-xgo branch from f299853 to 2224d34 Compare June 2, 2020 09:48
@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2020

Codecov Report

Merging #2318 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2318      +/-   ##
==========================================
+ Coverage   48.06%   48.11%   +0.05%     
==========================================
  Files         281      281              
  Lines       12989    12989              
==========================================
+ Hits         6243     6250       +7     
+ Misses       6147     6140       -7     
  Partials      599      599              
Impacted Files Coverage Δ
tequilapi/endpoints/sse_handler.go 72.17% <0.00%> (-2.61%) ⬇️
session/pingpong/invoice_tracker.go 66.13% <0.00%> (+0.63%) ⬆️
nat/traversal/pinger.go 72.33% <0.00%> (+3.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa40348...2224d34. Read the comment docs.

@zolia
Copy link
Contributor Author

zolia commented Jun 2, 2020

fixed by 924532c and do not depend on go-ethereum in supervisor. Closing this PR since major refactoring is needed here and best to move to mage format in order to eliminate excessive duplications.

@zolia zolia closed this Jun 2, 2020
@Waldz Waldz deleted the build-supervisor-with-xgo branch July 29, 2020 15:17
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.

PackageMacOS fails
7 participants