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

Allow configuration of submodule.fetchJobs and fetch.parallel #1569

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

Conversation

pecigonzalo
Copy link

@pecigonzalo pecigonzalo commented Dec 14, 2023

Resolves: #945

This PR aims to add support for fetchJobs and enable parallel submodule cloning by leveraging git config. Currently it uses --local config, but this could be switched to global.
This improves clone times for some repositories that rely on submodules (if network conditions allow).

The tests did not seem to be able to run on my fork, I don't know if this is how it works or some mistake I made, so I opened the PR directly here.

  • Add support for fetch.parallel

fi

echo "Testing fetchJobs exists"
git config --local --get-regexp submodules.fetchJobs | grep 10
Copy link
Author

Choose a reason for hiding this comment

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

Is this the right way to test this?

@@ -1,100 +1,104 @@
name: 'Checkout'
Copy link
Author

Choose a reason for hiding this comment

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

I don't know why this shows this big diff, I thought it could be formatting but it doesnt seem like it.

Copy link

Choose a reason for hiding this comment

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

If you want to see the reason, I would suggest to run git show --word-diff --word-diff-regex=. 2bd5ef5487b953b5db186c7a68dcab4e4a8fe3fc -- action.yml - you will see that it had to do with the line endings (maybe something like e.g. https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings can be of help?)

README.md Outdated
# integer allows up to that number of submodules fetched in parallel. A value of 0
# will give some reasonable default. If unset, it defaults to 1.
# Default: 1
submodulesFetchJobs: ''
Copy link
Author

Choose a reason for hiding this comment

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

I was not sure about the name, if submoduleFetchJobs (aligned with git config) or submodulesFetchJobs (aligned with the other variable here)

Choose a reason for hiding this comment

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

.github/workflows/npm-grunt.yml

Copy link
Author

Choose a reason for hiding this comment

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

Not sure what that references

@pecigonzalo pecigonzalo marked this pull request as ready for review December 14, 2023 14:00
@pecigonzalo pecigonzalo requested a review from a team as a code owner December 14, 2023 14:00
@pecigonzalo pecigonzalo changed the title Allow configuration of submodule.fetchJobs Allow configuration of submodule.fetchJobs and fetch.parallel Dec 14, 2023
@@ -153,6 +153,7 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {

// Fetch
core.startGroup('Fetching the repository')
await git.config('fetch.parallel', settings.fetchParallel.toString(), true)
Copy link
Author

Choose a reason for hiding this comment

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

Is this the right place to set it?

@EduMenges
Copy link

Any updates?

@pecigonzalo
Copy link
Author

Waiting for a maintainer to show 👍🏼 👎🏼 - I notice there are some conflicts, ill resolve them if I get the greenlight here.

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.

Parallel submodules update (--jobs <n>)
4 participants