application-research / autoretrieve

A server to make GraphSync data accessible on IPFS
22 stars 7 forks source link

fix: gh actions to work on both macos architectures #100

Closed rvagg closed 2 years ago

codecov-commenter commented 2 years ago

Codecov Report

Merging #100 (f0c7abf) into master (2d023c0) will decrease coverage by 0.64%. The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #100      +/-   ##
=========================================
- Coverage    8.04%   7.39%   -0.65%     
=========================================
  Files          12      12              
  Lines        1592    1865     +273     
=========================================
+ Hits          128     138      +10     
- Misses       1460    1724     +264     
+ Partials        4       3       -1     
Impacted Files Coverage Δ
config.go 0.00% <0.00%> (ø)
autoretrieve.go 0.00% <0.00%> (ø)
main.go 1.25% <0.00%> (+1.25%) :arrow_up:
blocks/manager.go 85.00% <0.00%> (+5.00%) :arrow_up:

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 2d023c0...f0c7abf. Read the comment docs.

elijaharita commented 2 years ago

is it necessary to pin the nightly version?

rvagg commented 2 years ago

Yeah, because it has to match what filecoin-ffi is built with.

BUT I'm trying to recall now why I needed the whole toolchain in here in the first place for macos. filecoin-ffi should be producing prebuilds for the environments we care about, including macos. For some reason when I was doing the CI stuff originally I found I needed it, but now I'm experimenting with it again I can't see why it's needed.

My guess is that maybe when I was testing the original CI install that I messed with the filecoin-ffi branch / commit stuff in a way that meant it didn't recognise it could fetch prebuilds for that version so it was needing to compile from source?

So what the latest change here does is:

  1. Moves the prep steps into Makefile so it's not tucked away in actions (if we need to put the rust toolchain stuff back it can go in there and be documented properly).
  2. Removes the rustup install and installation of cargo-lipo, which are only required for building filecoin-ffi from source.

@hannahhoward do you know of any reason that we should be prepared to build filecoin-ffi from source in CI?