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

Add common/platforms as git submodule #239

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Myrausman
Copy link

In order to address issue #70
1.Removed old common/platforms folder
2.Added openfasoc-platforms repo as submodule in the common/platforms folder

@msaligane
Copy link
Member

@Myrausman Thanks! Let's see if the CI passes.

@saicharan0112
Copy link
Collaborator

@Myrausman the CI is failing (though it is showing it as passed). These generators read config.mk files inside platforms directories. If you look at the check, we just clone this github repo. Can you share the command on how to checkout this module before running the flow please?

@Myrausman
Copy link
Author

@saicharan0112 , The follwoing command should help ensure that the submodule you mentioned is properly checked out before running the flow.
git submodule update --init --recursive --remote

@saicharan0112
Copy link
Collaborator

Ok, but can you please add the module to this list https://github.com/idea-fasoc/OpenFASOC/blob/main/.gitmodules ?

@Myrausman
Copy link
Author

I did enter module to this list as you can check here: https://github.com/Myrausman/OpenFASOC/blob/issue-70/.gitmodules

@saicharan0112
Copy link
Collaborator

I did enter module to this list as you can check here: https://github.com/Myrausman/OpenFASOC/blob/issue-70/.gitmodules

Perfect!

@Myrausman
Copy link
Author

@saicharan0112 ,I would appreciate it if you could provide some insights or updates regarding the current status of the pull request. Are there any additional changes or actions required from my end to move it forward?

@saicharan0112
Copy link
Collaborator

@Myrausman really appreciate your patience. This directory is used by all generators so I think its good to verify once with everybody what they think this change will effect the generators. However I don't think there are any changes from your side afais. Let me talk to others if that's ok with you

Copy link
Collaborator

@saicharan0112 saicharan0112 left a comment

Choose a reason for hiding this comment

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

@Myrausman : the platforms repository needs some restructuring, this PR will take care of it idea-fasoc/platforms#2. I updated workflows files to use the submodules after checkout of the repository.
Thanks for your contribution.

@Myrausman
Copy link
Author

okay, thank you

@msaligane
Copy link
Member

@alibillalhammoud @chetanyagoyal @nicholas this PR aims to add the platform folders as submodule which will reduce the repo size by a lot. Can you please check this?

@alibillalhammoud
Copy link
Collaborator

I am not to experienced with the github CI. I tried cloning the fork and running the following:

git clone git@github.com:Myrausman/OpenFASOC.git
git submodule update --init --recursive

For some reason this only results in LICENSE and a .git file in platforms folder. I would expect the whole platforms to be copied in. Perhaps I am running this wrong?

@@ -5,3 +5,6 @@
path = tapeouts
url = https://github.com/idea-fasoc/openfasoc-tapeouts.git
branch = main
[submodule "openfasoc/common/platforms"]
path = openfasoc/common/platforms
url = https://github.com/idea-fasoc/platforms
Copy link
Collaborator

Choose a reason for hiding this comment

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

The platforms repo has a folder common/platforms in it. Does this mean the submodule will initialize in common/platforms/common/platforms? @Myrausman

@msaligane
Copy link
Member

@harshkhandeparkar The CI will fail after merging this? Have you thought this through? are the checks updated?

@harshkhandeparkar
Copy link
Collaborator

@harshkhandeparkar The CI will fail after merging this? Have you thought this through? are the checks updated?

idea-fasoc/platforms#2 This PR has to be merged first. The directory structure was wrong...

@harshkhandeparkar
Copy link
Collaborator

@chetanyagoyal can you please check why this is failing or whether it is intended behaviour?

@chetanyagoyal
Copy link
Collaborator

@chetanyagoyal can you please check why this is failing or whether it is intended behaviour?

will check

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.

6 participants