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

Update modules to support riscv64 #39423

Merged
merged 1 commit into from
Jul 2, 2019
Merged

Update modules to support riscv64 #39423

merged 1 commit into from
Jul 2, 2019

Conversation

carlosedp
Copy link
Contributor

Signed-off-by: CarlosEDP me@carlosedp.com

Updating projects using libraries that were recently updated to support Risc-V architecture.

Go upstream work is tracked on: golang/go#27532

Risc-V software support tracker on https://github.com/carlosedp/riscv-bringup

@tiborvass
Copy link
Contributor

@carlosedp you inadvertently added the dockerd binary in the PR.

@carlosedp
Copy link
Contributor Author

My bad, it fell into the cracks together with all the files. Should be removed now.

@tiborvass
Copy link
Contributor

tiborvass commented Jun 26, 2019

@carlosedp the vendor check failed, you've got to vendor this way:

$ make BIND_DIR=. shell
# vndr

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "riscv64" git@github.com:carlosedp/moby.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354512336
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jun 26, 2019
@codecov
Copy link

codecov bot commented Jun 26, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@e105a74). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #39423   +/-   ##
=========================================
  Coverage          ?   37.33%           
=========================================
  Files             ?      609           
  Lines             ?    45197           
  Branches          ?        0           
=========================================
  Hits              ?    16875           
  Misses            ?    26034           
  Partials          ?     2288

@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jun 26, 2019
@carlosedp
Copy link
Contributor Author

@tiborvass Why when I deleted whole vendor dir and ran vndr it didn't do the correct thing?

When I ran inside the shell, the difference was that some go.mod files were deleted.

@tiborvass
Copy link
Contributor

@carlosedp probably vndr version differences, that's why we pin the vndr version in the dockerfile so that everyone agrees on how to vendor.

@thaJeztah
Copy link
Member

reminds me that we need to debug why updating to the latest vndr version causes CI to fail; #39216

@thaJeztah
Copy link
Member

oh; before we merge this one, could we merge #39054 ? in case we want to backport that one (without introducing the new things from the other bumps)

we also might want to ask if they can tag a new version for netlink

@tiborvass
Copy link
Contributor

tiborvass commented Jun 28, 2019

Let's also get #39216 first, it includes a vndr that has riscv64 support (LK4D4/vndr#80) I can rebase this PR then.

vendor.conf Show resolved Hide resolved
@carlosedp carlosedp requested a review from tianon as a code owner June 29, 2019 18:00
@tiborvass
Copy link
Contributor

@carlosedp I updated your PR. One notable difference is that the netns commit was 13995c7128ccc8e51e9a6bd2b551020a27180abd which didn't include the riscv changes from vishvananda/netns#34. So I updated it to 7109fa855b0ff1ebef7fbd2f6aa613e8db7cfbc0.

@carlosedp
Copy link
Contributor Author

Amazing. Thanks @tiborvass. Now the vndr I used standalone would work fine since the CI have been updated as well right?

@tiborvass
Copy link
Contributor

@carlosedp most likely, but I'd still recommend using make BIND_DIR=. shell just in case. Either way, the vendor CI will catch any mismatches.

For the record, I couldn't figure out this: #39423 (comment)

@carlosedp
Copy link
Contributor Author

After updating libnetwork it failed building. I saw the latest libnetwork update required a new sctp too.

@tiborvass
Copy link
Contributor

Ugh, forgot to include moby/libnetwork#2412 so I updated libnetwork as well.

@thaJeztah
Copy link
Member

vendor check is failing @tiborvass

Signed-off-by: CarlosEDP <me@carlosedp.com>
@tiborvass
Copy link
Contributor

@thaJeztah what are you talking about? 👀 😛jk updated

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael
Copy link
Contributor

LGTM

@crosbymichael crosbymichael merged commit 527f9f7 into moby:master Jul 2, 2019
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.

None yet

5 participants