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.

Sign up for GitHub

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

Jump to bottom

Expand environment variables for sparse checkout path #1066

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+58 −10
Open

Expand environment variables for sparse checkout path #1066

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85254b1
Expand environment variables in the sparse checkout paths.
raghavgarg098 Apr 8, 2021
a351003
Merge branch 'master' into SparseCheckout
raghavgarg098 Apr 8, 2021
b84b46f
Removed line spaces and unintended changes.
raghavgarg098 Apr 8, 2021
cb0e9af
Fixed the failing tests.
raghavgarg098 Apr 9, 2021
182e722
Merge branch 'master' into SparseCheckout
MarkEWaite Jul 24, 2021
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
44 changes: 34 additions & 10 deletions src/main/java/hudson/plugins/git/extensions/impl/SparseCheckoutPath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,25 @@
import com.google.common.base.Function;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.EnvVars;
import hudson.Extension;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nullable;
import java.io.Serializable;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class SparseCheckoutPath extends AbstractDescribableImpl<SparseCheckoutPath> implements Serializable {

private static final long serialVersionUID = -6177158367915899356L;

@SuppressFBWarnings(value="SE_TRANSIENT_FIELD_NOT_RESTORED", justification="Default value is OK in deserialization")
public static final transient SparseCheckoutPathToPath SPARSE_CHECKOUT_PATH_TO_PATH = new SparseCheckoutPathToPath();
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this change break compatibility for git plugin API consumers that refer to this public field that is being deleted?

It is included in the Javadoc of the current release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to keep this variable for API consumers.

@SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "Default value is OK in deserialization")

private final String path;

Expand Down Expand Up @@ -56,20 +59,41 @@ public String toString() {
return path;
}

private static class SparseCheckoutPathToPath implements Function<SparseCheckoutPath, String>, Serializable {
public String apply(@NonNull SparseCheckoutPath sparseCheckoutPath) {
return sparseCheckoutPath.getPath();
}
public Descriptor<SparseCheckoutPath> getDescriptor() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't make white space changes in the git plugin. The formatting is ugly and inconsistent, but changing white space makes it more difficult to read the changes and more difficult to interact with other proposed changes in the same file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why the getDescriptor() call moved outside SparseCheckoutPathToPath. What makes that move necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not intentional. Somehow changed on reformating the code in IDE.

return Jenkins.get().getDescriptor(getClass());
}

public Descriptor<SparseCheckoutPath> getDescriptor()
{
return Jenkins.get().getDescriptor(getClass());
public static class SparseCheckoutPathToPath implements Function<SparseCheckoutPath, String>, Serializable {
@Nullable
private EnvVars envVars;

SparseCheckoutPathToPath(@Nullable EnvVars envVars) {
this.envVars = envVars;
}

public String apply(@NonNull SparseCheckoutPath sparseCheckoutPath) {
String path = sparseCheckoutPath.getPath();
if (envVars == null) {
return path;
}

// Pattern to look for substring of the form ${ENV_VAR}.
Pattern pattern = Pattern.compile("\\$\\{(.*?)\\}");
Matcher matcher = pattern.matcher(path);
while (matcher.find()) {
String varName = matcher.group(1);
String value = envVars.get(varName, "");
path = path.replace("${" + varName + "}", value);
}
return path;
}
}

@Extension
public static class DescriptorImpl extends Descriptor<SparseCheckoutPath> {
@Override
public String getDisplayName() { return "Path"; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't make white space only changes in the git plugin. See the contributing file for more details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned this was not intentional either.

public String getDisplayName() {
return "Path";
}
}
}
31 changes: 16 additions & 15 deletions src/main/java/hudson/plugins/git/extensions/impl/SparseCheckoutPaths.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
package hudson.plugins.git.extensions.impl;

import com.google.common.collect.Lists;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.plugins.git.GitException;
import hudson.plugins.git.GitSCM;
import hudson.plugins.git.extensions.GitSCMExtension;
import hudson.plugins.git.extensions.GitSCMExtensionDescriptor;
import hudson.plugins.git.extensions.impl.SparseCheckoutPath.SparseCheckoutPathToPath;
import org.jenkinsci.plugins.gitclient.CheckoutCommand;
import org.jenkinsci.plugins.gitclient.CloneCommand;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.jenkinsci.plugins.gitclient.UnsupportedCommand;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.kohsuke.stapler.DataBoundConstructor;
import edu.umd.cs.findbugs.annotations.NonNull;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -36,27 +37,19 @@ public List<SparseCheckoutPath> getSparseCheckoutPaths() {

@Override
public void decorateCloneCommand(GitSCM scm, Run<?, ?> build, GitClient git, TaskListener listener, CloneCommand cmd) throws IOException, InterruptedException, GitException {
if (! sparseCheckoutPaths.isEmpty()) {
if (!sparseCheckoutPaths.isEmpty()) {
listener.getLogger().println("Using no checkout clone with sparse checkout.");
}
}

@Override
public void decorateCheckoutCommand(GitSCM scm, Run<?, ?> build, GitClient git, TaskListener listener, CheckoutCommand cmd) throws IOException, InterruptedException, GitException {
cmd.sparseCheckoutPaths(Lists.transform(sparseCheckoutPaths, SparseCheckoutPath.SPARSE_CHECKOUT_PATH_TO_PATH));
cmd.sparseCheckoutPaths(Lists.transform(sparseCheckoutPaths, new SparseCheckoutPathToPath(build.getEnvironment(listener))));
}

@Override
public void determineSupportForJGit(GitSCM scm, @NonNull UnsupportedCommand cmd) {
cmd.sparseCheckoutPaths(Lists.transform(sparseCheckoutPaths, SparseCheckoutPath.SPARSE_CHECKOUT_PATH_TO_PATH));
}

@Extension
public static class DescriptorImpl extends GitSCMExtensionDescriptor {
@Override
public String getDisplayName() {
return "Sparse Checkout paths";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why moving so many methods outside the descriptor implementation class? Seems like that will break many things and add unused public methods to the SparseCheckoutPaths object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same

cmd.sparseCheckoutPaths(Lists.transform(sparseCheckoutPaths, new SparseCheckoutPathToPath(null)));
}

/**
Expand All @@ -67,11 +60,11 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not able to resolve these issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not able to revert these changes.


if (o == null || getClass() != o.getClass()) {
return false;
}

SparseCheckoutPaths that = (SparseCheckoutPaths) o;
return Objects.equals(getSparseCheckoutPaths(), that.getSparseCheckoutPaths());
}
Expand All @@ -83,7 +76,7 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(getSparseCheckoutPaths());
}

/**
* {@inheritDoc}
*/
Expand All @@ -93,4 +86,12 @@ public String toString() {
"sparseCheckoutPaths=" + sparseCheckoutPaths +
'}';
}

@Extension
public static class DescriptorImpl extends GitSCMExtensionDescriptor {
@Override
public String getDisplayName() {
return "Sparse Checkout paths";
}
}
}
22 changes: 22 additions & 0 deletions src/test/java/hudson/plugins/git/extensions/impl/SparseCheckoutPathsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
package hudson.plugins.git.extensions.impl;

import hudson.EnvVars;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Level;
Expand All @@ -40,9 +41,12 @@
import nl.jqno.equalsverifier.EqualsVerifier;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;

import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class SparseCheckoutPathsTest {

Expand Down Expand Up @@ -122,6 +126,24 @@ public void testDecorateCheckoutCommand() throws Exception {
assertThat(cmd.getSparsePathNames(), hasItems(SRC_DIR_NAME));
}

@Test
public void testDecorateCheckoutCommandExpandsEnvVariable() throws Exception {
GitSCM scm = null;
GitClient git = null;
Run<?, ?> build = mock(Run.class);
EnvVars envVars = new EnvVars();
envVars.put("SPARSE_CHECKOUT_DIRECTORY", SRC_DIR_NAME);
when(build.getEnvironment(listener)).thenReturn(envVars);

MyCheckoutCommand cmd = new MyCheckoutCommand();
List<SparseCheckoutPath> sparseCheckoutPathList = new ArrayList<>();
sparseCheckoutPathList.add(new SparseCheckoutPath("${SPARSE_CHECKOUT_DIRECTORY}"));
SparseCheckoutPaths sparseCheckoutPaths = new SparseCheckoutPaths(sparseCheckoutPathList);
sparseCheckoutPaths.decorateCheckoutCommand(scm, build, git, listener, cmd);

assertThat(cmd.getSparsePathNames(), hasItems(SRC_DIR_NAME));
}

@Test
public void equalsContract() {
EqualsVerifier.forClass(SparseCheckoutPaths.class).usingGetClass().verify();
Expand Down