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

efibuild: Add support for DISCARD_SUBMODULES #24

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

mikebeaton
Copy link
Contributor

@mikebeaton mikebeaton commented Nov 8, 2023

This is faster to build than using DISCARD_PACKAGES since OpenCorePkg does not need to be cloned (which is relatively slow) then deleted. To implement in OpenCore requires changing build_duet.tool and build_oc.tool to set and export DISCARD_SUBMODULES instead of DISCARD_PACKAGES. (Not sure if there are other projects that can be updated too, though DISCARD_PACKAGES is still supported.)

Requires new-enough git to work, though I believe that is not an issue here - unless we perhaps need to support building on user-owned machines too old to have this?

@mikebeaton
Copy link
Contributor Author

mikebeaton commented Nov 8, 2023

Have added this test commit https://github.com/mikebeaton/OpenCorePkg/commits/discard-test which directly uses efibuild.sh from this PR, and confirms that the full CI build works using this change.

@savvamitrofanov
Copy link
Contributor

This is faster to build than using DISCARD_PACKAGES since OpenCorePkg does not need to be cloned (which is relatively slow) then deleted. To implement in OpenCore requires changing build_duet.tool and build_oc.tool to set and export DISCARD_SUBMODULES instead of DISCARD_PACKAGES. (Not sure if there are other projects that can be updated too, though DISCARD_PACKAGES is still supported.)

Requires new-enough git to work, though I believe that is not an issue here - unless we perhaps need to support building on user-owned machines too old to have this?

LGTM, but I suggest to remove submodule in such way:

git submodule deinit <submodule-name>    
git rm <submodule-name>

It should be compatible across different git versions

@mikebeaton
Copy link
Contributor Author

mikebeaton commented Nov 12, 2023

I'm not sure if the git submodule deinit call is needed, because this code is called only when the submodules have not yet been initialised (at least it is now, with a submodules.ready guard file, which I have also added; so this discard code should only happen when the repo is being first checked out, before the first call to git submodule update --init).

So in the current version (git submodule deinit included) we get the following log messages:

error: could not lock config file .git/modules/OpenCorePkg/config: No such file or directory
warning: Could not unset core.worktree setting in submodule 'OpenCorePkg'
rm 'OpenCorePkg'
[master c0718d6] Discarded submodules

Not needed as we are removing before init
@mikebeaton
Copy link
Contributor Author

mikebeaton commented Nov 13, 2023

I've added a third commit, removing git submodule deinit again, but keeping the guard file so that this is definitely all only done once (before init). I think this is right. (Full build retested at https://github.com/mikebeaton/OpenCorePkg/tree/discard-test.)

@mikebeaton
Copy link
Contributor Author

@savvamitrofanov - Does the reasoning for not including git submodule deinit after all, and the addition of the guard file, look correct to you too?

@savvamitrofanov savvamitrofanov merged commit ac9922f into acidanthera:master Nov 16, 2023
@mikebeaton mikebeaton deleted the discard-submodules branch April 29, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants