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

[JENKINS-21248] Support shallow submodule update #303

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1052,9 +1052,11 @@ public SubmoduleUpdateCommand submoduleUpdate() {
boolean recursive = false;
boolean remoteTracking = false;
boolean parentCredentials = false;
boolean shallow = false;
String ref = null;
Map<String, String> submodBranch = new HashMap<>();
public Integer timeout;
public Integer depth = 1;

public SubmoduleUpdateCommand recursive(boolean recursive) {
this.recursive = recursive;
Expand Down Expand Up @@ -1086,6 +1088,16 @@ public SubmoduleUpdateCommand timeout(Integer timeout) {
return this;
}

public SubmoduleUpdateCommand shallow(boolean shallow) {
this.shallow = shallow;
return this;
}

public SubmoduleUpdateCommand depth(Integer depth) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible I would go for int instead of Integer for the method parameter and instance variable. (I also see boolean and no Boolean).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. I have seen now that this is done like this to be consisent with the methods for clone/fetch.

this.depth = depth;
return this;
}

/**
* @throws GitException if executing the Git command fails
* @throws InterruptedException if called methods throw same exception
Expand Down Expand Up @@ -1116,6 +1128,12 @@ else if (!referencePath.isDirectory())
else
args.add("--reference", ref);
}
if (shallow) {
if (depth == null){
depth = 1;
}
args.add("--depth=" + depth);
}


// We need to call submodule update for each configured
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,7 @@ public org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand submoduleUpdate()
return new org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand() {
boolean recursive = false;
boolean remoteTracking = false;
boolean shallow = false;
String ref = null;

@Override
Expand Down Expand Up @@ -2219,6 +2220,17 @@ public org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand timeout(Integer ti
return this;
}

@Override
public org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand shallow(boolean shallow) {
this.shallow = shallow;
return this;
}

@Override
public org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand depth(Integer depth) {
return this;
}

@Override
public org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand useBranch(String submodule, String branchname) {
return this;
Expand All @@ -2234,6 +2246,10 @@ public void execute() throws GitException, InterruptedException {
listener.getLogger().println("[ERROR] JGit doesn't support submodule update --reference yet.");
throw new UnsupportedOperationException("not implemented yet");
}
if (shallow) {
listener.getLogger().println("[ERROR] JGit doesn't support shallow submodules yet.");
throw new UnsupportedOperationException("not implemented yet");
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @MarkEWaite, should using shallow for JGit throw this UnsupportedOperationException here or just log a WARNING when calling shallow(true) at lines 2224-2227?

As a reference FetchCommand and CloneCommand just perform logging. That's not consistent in this class.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right @darxriggs , it would be good enough to log the implementation gap rather than fail the checkout. Shallow clone is a performance optimization that isn't critical to the typical use of the repository.

}

try (Repository repo = getRepository()) {
SubmoduleUpdateCommand update = git(repo).submoduleUpdate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,21 @@ public interface SubmoduleUpdateCommand extends GitCommand {
* @return a {@link org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand} object.
*/
SubmoduleUpdateCommand timeout(Integer timeout);

/**
* shallow.
*
* @param shallow a boolean.
* @return a {@link org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand} object.
*/
SubmoduleUpdateCommand shallow(boolean shallow);

/**
* When shallow cloning, allow for a depth to be set in cases where you need more than the immediate last commit.
* Has no effect if shallow is set to false (default)
*
* @param depth number of revisions to be included in shallow clone
* @return a {@link org.jenkinsci.plugins.gitclient.SubmoduleUpdateCommand} object.
*/
SubmoduleUpdateCommand depth(Integer depth);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2575,6 +2575,18 @@ public void test_submodule_update() throws Exception {
assertFixSubmoduleUrlsThrows();
}

@NotImplementedInJGit
public void test_submodule_update_shallow() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Other test cases would also be useful:

  • no shallow(...) call
  • shallow(false)
  • depth(...)

w.init();
w.git.clone_().url(localMirror()).repositoryName("sub2_origin").execute();
w.git.checkout().branch("tests/getRelativeSubmodule").ref("sub2_origin/tests/getRelativeSubmodule").deleteBranchIfExist(true).execute();
w.git.submoduleInit();
w.git.submoduleUpdate().shallow(true).execute();

final String shallow = ".git" + File.separator + "modules" + File.separator + "sub" + File.separator + "shallow";
assertTrue("Shallow file does not exist: " + shallow, w.exists(shallow));
}

@NotImplementedInJGit
public void test_trackingSubmoduleBranches() throws Exception {
if (! ((CliGitAPIImpl)w.git).isAtLeastVersion(1,8,2,0)) {
Expand Down