Skip to content

Removing black-parrot-tools and black-parrot-sdk#2660

Open
dpetrisko wants to merge 5 commits into
chipsalliance:mainfrom
dpetrisko:main
Open

Removing black-parrot-tools and black-parrot-sdk#2660
dpetrisko wants to merge 5 commits into
chipsalliance:mainfrom
dpetrisko:main

Conversation

@dpetrisko
Copy link
Copy Markdown

These don't actually do anything for the synlig built, but take a lot of space and time to clone

@hzeller
Copy link
Copy Markdown
Collaborator

hzeller commented Nov 18, 2024

Drive-by comment: Maybe in the future the tests should be somewhat separate from submodules; have a file with a list of git URLs and commit-hashes that are fetched when tests are run, but not by default as submodule (as submodule sortof imply that it is a necessary build-resource).

@pgielda
Copy link
Copy Markdown
Member

pgielda commented Nov 18, 2024

I think the reason is the bot that tries to autoupdate those submodules...

@hzeller
Copy link
Copy Markdown
Collaborator

hzeller commented Nov 18, 2024

but could it also just auto-update a file with <git-url> <hash> tuples ?

@pgielda
Copy link
Copy Markdown
Member

pgielda commented Nov 18, 2024

I was not the one that set it up, it was also just a drive-by comment ;) Maybe @kgugala knows if dependabot can handle anything else than submodules.

Note that the CI does git submodule sync && git submodule update --init --recursive third_party/{surelog,yosys} so it goes around the problem this way.

@pgielda
Copy link
Copy Markdown
Member

pgielda commented Nov 18, 2024

(note that this MR cannot be merged as-is because it makes black parrot tests red)

@dpetrisko
Copy link
Copy Markdown
Author

Oh, I see there’s a useless Makefile dependency in BP to the tools and sdk. I’ll push a fix and then bump BP to fix it

@dpetrisko
Copy link
Copy Markdown
Author

dpetrisko commented Nov 21, 2024

Okay, I think that bump should make tests green

@kgugala
Copy link
Copy Markdown
Member

kgugala commented Nov 22, 2024

I believe you would also need to remove the BP tests (as those failed here).

Why do you need to drop the submodule? All the optional submodules (the ones we use for testing, but not required to build the tool) are marked with update = none (see https://github.com/chipsalliance/synlig/blob/main/.gitmodules#L34). This means that those submodules are not fetched if you run git submodule update --init --recursive. To fetch the optional submodule you have to explicitly point it. In CI we do it with this script https://github.com/chipsalliance/synlig/blob/main/tests/scripts/common.sh#L25 called from https://github.com/chipsalliance/synlig/blob/main/tests/scripts/run_large_designs.sh#L117

@pgielda
Copy link
Copy Markdown
Member

pgielda commented Nov 22, 2024

why would we be removing BP tests?

@dpetrisko
Copy link
Copy Markdown
Author

I think the tests failed as result of the bump, looks the patch is no longer valid. Will investigate

@dpetrisko
Copy link
Copy Markdown
Author

Hm not sure why the ASIC synthesis fails if the other two are working. Any insights?

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.

4 participants