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

Script to checkout correct versions of submodules #424

Closed
diyelectromusic opened this issue Jan 22, 2023 · 14 comments · Fixed by #475
Closed

Script to checkout correct versions of submodules #424

diyelectromusic opened this issue Jan 22, 2023 · 14 comments · Fixed by #475

Comments

@diyelectromusic
Copy link
Contributor

I think it would ease development if there was a script, maybe alongside build.sh, that checks out the same versions of submodules as are used in the github actions.

Of course there may be much better ways to achieve this in GitHub (I'm no GitHub/git expert), so it doesn't have to be done this way - but I think we should have a simple way to ensure anyone contributing code is starting from the same versions built into the releases.

Kevin

@probonopd
Copy link
Owner

Good idea. Basically we could move

cd circle-stdlib/
git checkout e318f89 # Needed to support Circle develop?
cd -
cd circle-stdlib/libs/circle
git checkout ec09d7e # develop
cd -
cd circle-stdlib/libs/circle-newlib
git checkout 48bf91d # needed for circle ec09d7e

to its own script that could also be run locally.
Wdyt?

@diyelectromusic
Copy link
Contributor Author

Well I must confess I don't really understand how the whole things hang together (yet), so whatever you think works best. As long as it won't overwrite any local changes in progress that could be an option.

I suppose I was thinking the build is something that is done quite regularly as part as normal development, but the checkout and initialisation of any specific version of the repository is something only done at specific controlled points.

I guess the ideal would be that updating a fork means it gets updated with the right versions of submodules automatically, but I don't know how the versions listed in the code itself relate to the versions in build.yml...

Kevin

@trygvelu
Copy link

I don't think this should be necessary. Running 'git submodule update --recursive' after checking out the main project should check out the referenced commits in the submodules, provided that the references to the submodules' commits have also been committed in the main project and "chain" of submodules. That is, whenever a developer has updated a submodule in the project in order to bring in new functionality, squash bugs etc, then: git add/commit the submodule in the project to bring in the new commit/state of the submodule.

Creating a script that checks out specific commits sounds like extra work to me - how often would the references/script be updated? IMO the build workflow should just check out from origin/main branch, then update submodules and proceed with building from there.

However - I fully agree that it should be easier to build MiniDexed. I had to fiddle quite a bit to build a functioning version.

@probonopd
Copy link
Owner

probonopd commented Jan 24, 2023

Thing is, it is a pain in the * to update git revisions on GitHub (e.g., switch Circle to another develop git commit). The only pragmatic solution I found so far is the one that is in place now. I agree that it's not optimal. But we do need full control over which versions/commits of which dependencies are being used, or else the result will not be fully reproducible.

@diyelectromusic
Copy link
Contributor Author

Any further thoughts on this one? When setting up a new MiniDexed source tree for the latest round of updates I was doing all this by hand again... :)

It would be really nice to just run a "refresh" or "resync" type action and have it all ready to go again. But very happy to be told there is a better way!

Kevin

diyelectromusic added a commit to diyelectromusic/MiniDexed that referenced this issue Apr 4, 2023
@diyelectromusic
Copy link
Contributor Author

Ok, I've had a go at creating a simple script to mirror the build steps from the git workflow, but I've not submittted a proper PR. See what you think: https://github.com/diyelectromusic/MiniDexed/tree/Issue424Fix

We'd need to get the build.yml script to call out to this rather than try to maintain everything in two places.

On a related note, is there a reason why build.yml doesn't; use the build.sh script for each of the builds? Again that feels like it is duplicating information in two different places?

Kevin

@probonopd
Copy link
Owner

Thanks Kevin, but as long as we don't also change git GitHub yml your new script won't be used. So let's also change the GitHub yml so that we don't have to update this in two places.

On a related note, is there a reason why build.yml doesn't; use the build.sh script

I don't remember all the details but as long as it's working I won't touch it again ;-)

@diyelectromusic
Copy link
Contributor Author

Indeed, but I'm not sure I want to go near the build.yml file, so I'll leave that one to you!!

I do think we ought to call out to build.sh too so that developers and github are building everything the same way, but I can understand the reluctance to touch it!

Kevin

probonopd pushed a commit that referenced this issue Apr 7, 2023
* Fix for issue #424 create a build script to check out the correct versions of sub libraries.

* Initial attempt at supporting loading SysEx files from subdirectories which seems to speed up loading significantly.

* Reinstate headerless SysEx file loading after subdirectory change

* Limit nesting of subdirectories to open

* Fix bank number in UI to match the numbers from the bank files.

* Update to fix bank numbers.  Bank file numbers now start from 00001 and show as 1-indexed in the UI.  But internally and via MIDI are still 0-indexed as per MIDI spec.
@diyelectromusic
Copy link
Contributor Author

Pulling in PR #473 has included this script now (sorry!). But if you're ok with that, then could you take a look at then how to get build.yml to call it? I've no idea how any of the GitHub action stuff works.

And I still think it would also be good to call out to build.sh for each of the Pi builds too to keep that all consistent too.

Kevin

@diyelectromusic
Copy link
Contributor Author

diyelectromusic commented Apr 7, 2023

And I still think it would also be good to call out to build.sh for each of the Pi builds too to keep that all consistent too.

Oh ignore that - looks like it does! Each of the lines like the following are doing it:
RPI=4 bash -ex build.sh

So it might just be a case of replacing the appropriate bit in build.yml with the following:

run: |
set -ex
bash -ex submod.sh

but I'm just guessing - as I say, I don't really know what I'm talking about!?

Kevin

@probonopd
Copy link
Owner

I think you are right, let me try it out.

@probonopd
Copy link
Owner

Thank you very much @diyelectromusic - it works.

@diyelectromusic
Copy link
Contributor Author

Yay!

Probably need to update the developer section of the wiki now.
Aside: also, maybe we should do something similar for Synth_Dexed...

:)

@probonopd
Copy link
Owner

probonopd commented Apr 7, 2023

Yes, and yes:

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 a pull request may close this issue.

3 participants