Skip to content

Conversation

macladson
Copy link
Member

@macladson macladson commented Aug 18, 2022

Issue Addressed

Recent changes to the Nethermind codebase removed the rocksdb git submodule in favour of a nuget package.
This appears to have broken our ability to build the latest release of Nethermind inside our integration tests.

Proposed Changes

Temporarily pin the version used for the Nethermind integration tests to master. This ensures we use the packaged version of rocksdb. This is only necessary until a new release of Nethermind is available.

Use git submodule update --init --recursive to ensure the required submodules are pulled before building.

@macladson macladson added the ready-for-review The code is ready for review label Aug 18, 2022
@macladson macladson changed the title Unblock CI by pinning Nethermind integration to master Unblock CI by pinning Nethermind integration tests to master Aug 18, 2022
@divagant-martian
Copy link
Contributor

divagant-martian commented Aug 18, 2022

I think we can change our routine to add resilience to these changes by

  1. removing the --recusive from the clone
  2. after checkout, doing git submodule update --init --recursive. It should bring the right submodules in the right commits.

@divagant-martian divagant-martian added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 18, 2022
@divagant-martian
Copy link
Contributor

divagant-martian commented Aug 18, 2022

I tried this in this here #3481 to be sure it would work. here is there relevant commit 684177c
Marks as failed because I just cancelled other tests

whitespace
@macladson macladson force-pushed the fix-nethermind-rocksdb branch from 15ec011 to e8e3457 Compare August 19, 2022 01:14
@macladson
Copy link
Member Author

Nice that's a better solution, I've reverted my changes and cherry-picked your commit, thanks! 🙏

@macladson macladson changed the title Unblock CI by pinning Nethermind integration tests to master Unblock CI by updating git submodules directly in execution integration tests Aug 19, 2022
@macladson macladson added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 19, 2022
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

I have verified this works as expected locally, CI seems to be passing as well.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. v3.0.0 🐼 Required for the v3.0.0 release and removed ready-for-review The code is ready for review labels Aug 19, 2022
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 19, 2022
…on tests (#3479)

## Issue Addressed

Recent changes to the Nethermind codebase removed the `rocksdb` git submodule in favour of a `nuget` package.
This appears to have broken our ability to build the latest release of Nethermind inside our integration tests.

## Proposed Changes

~Temporarily pin the version used for the Nethermind integration tests to `master`. This ensures we use the packaged version of `rocksdb`. This is only necessary until a new release of Nethermind is available.~

Use `git submodule update --init --recursive` to ensure the required submodules are pulled before building.

Co-authored-by: Diva M <divma@protonmail.com>
@bors bors bot changed the title Unblock CI by updating git submodules directly in execution integration tests [Merged by Bors] - Unblock CI by updating git submodules directly in execution integration tests Aug 19, 2022
@bors bors bot closed this Aug 19, 2022
@michaelsproul michaelsproul mentioned this pull request Aug 19, 2022
@macladson macladson deleted the fix-nethermind-rocksdb branch January 27, 2023 18:19
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…on tests (sigp#3479)

## Issue Addressed

Recent changes to the Nethermind codebase removed the `rocksdb` git submodule in favour of a `nuget` package.
This appears to have broken our ability to build the latest release of Nethermind inside our integration tests.

## Proposed Changes

~Temporarily pin the version used for the Nethermind integration tests to `master`. This ensures we use the packaged version of `rocksdb`. This is only necessary until a new release of Nethermind is available.~

Use `git submodule update --init --recursive` to ensure the required submodules are pulled before building.

Co-authored-by: Diva M <divma@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v3.0.0 🐼 Required for the v3.0.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants