Skip to content
This repository has been archived by the owner on Feb 26, 2023. It is now read-only.

Feature lighter init #44

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feature lighter init #44

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
12f0c00
shallow fetch
astik Nov 13, 2016
c707228
permanent git folder
astik Nov 13, 2016
7310e8a
remove permanent flag (check path inside method) and reverse check fo…
astik Nov 15, 2016
14173b1
permanentRoot settings overrides safeCheckout
astik Nov 26, 2016
1062c2f
documentation
astik Nov 26, 2016
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
14 changes: 12 additions & 2 deletions src/main/java/ar/com/synergian/wagongit/GitBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import java.io.File;
import java.io.IOException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please remove unneeded imports?


import org.apache.maven.scm.ScmException;
import org.apache.maven.scm.log.ScmLogger;
Expand All @@ -17,6 +20,7 @@ public class GitBackend {
final File workDir;
private final String remote;
private final String branch;
private final boolean enableShallowFetch;

private final ScmLogger log;

Expand All @@ -40,13 +44,14 @@ public void consumeLine(String line) {

};

public GitBackend(File workDir, String remote, String branch, ScmLogger log) throws GitException {
public GitBackend(File workDir, String remote, String branch, ScmLogger log, boolean enableShallowFetch) throws GitException {
this.log = log;
this.remote = remote;
this.branch = branch;
this.workDir = workDir;
if (!this.workDir.exists())
throw new GitException("Invalid directory");
this.enableShallowFetch = enableShallowFetch;

}

Expand Down Expand Up @@ -115,8 +120,13 @@ public void pullAll() throws GitException {
}
}

if (!run("fetch", new String[] { "--progress" }))
String[] fetchArgs = new String[] { "--progress" };
if (enableShallowFetch) {
fetchArgs = new String[] { "--progress", "--depth=1" };
}
if (!run("fetch", fetchArgs)) {
throw new GitException("git fetch failed");
}
}

// if remote branch doesn't exist, create new "headless".
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/ar/com/synergian/wagongit/GitWagon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public class GitWagon extends StreamWagon {
private final boolean debug = Utils.getBooleanEnvironmentProperty("wagon.git.debug");
private final boolean safeCheckout = Utils.getBooleanEnvironmentProperty("wagon.git.safe.checkout");
private final boolean skipEmptyCommit = Utils.getBooleanEnvironmentProperty("wagon.git.skip.empty.commit");
private final boolean enableShallowFetch = Utils.getBooleanEnvironmentProperty("wagon.git.enable.shallow.fetch");
Copy link
Owner

Choose a reason for hiding this comment

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

Can we invert logic here? What about disableShallowFetch thus we would assume it's enabled by default?

private final String permanentRoot = Utils.getStringEnvironmentProperty("wagon.git.permanent.root");

private final ScmLogger log = new GitWagonLog(debug);

Expand Down Expand Up @@ -116,15 +118,20 @@ protected void openConnectionInternal() throws ConnectionException, Authenticati
remote = url.substring(i + 3, url.length());
}

File workDir = Utils.createCheckoutDirectory(remote);
File workDir;
if (permanentRoot != null && !"".equals(permanentRoot)) {
workDir = Utils.createCheckoutDirectory(permanentRoot, true);
} else {
workDir = Utils.createCheckoutDirectory(remote, false);
}

if (!workDir.exists() || !workDir.isDirectory() || !workDir.canWrite())
throw new ConnectionException("Unable to create working directory");

if (safeCheckout)
FileUtils.cleanDirectory(workDir);

git = new GitBackend(workDir, remote, branch, log);
git = new GitBackend(workDir, remote, branch, log, enableShallowFetch);
git.pullAll();
} catch (Exception e) {
throw new ConnectionException("Unable to pull git repository: " + e.getMessage(), e);
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/ar/com/synergian/wagongit/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ public final class Utils {
private Utils() {
}

public static File createCheckoutDirectory(String path) throws GitException {

File dir = new File(System.getProperty("java.io.tmpdir"), "wagon-git-" + hashPath(path));
public static File createCheckoutDirectory(String path, boolean isPermanent) throws GitException {
File dir;
if (isPermanent) {
dir = new File(path);
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not totally happy with this change. Let's forget path for a while, path is just a text transformation of the repo URI. What if the new argument just replaces the new File(java.io.tempdir) if present (not null)?

Copy link
Author

@astik astik Nov 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure of what you mean =/
Remote is not passed to Utils (it is defined inside GitBackend which calls Utils). So without passing more arguments to the function, apart from the current proposition, i don't see how i can compute the temp file (from remote) and override it with the new argument.
Do you mean something like this :

    public static File createCheckoutDirectory(String remote, String overridenPath) throws GitException {
        File dir;
        if (overridenPath != null && !"".equals(overridenPath)) {
            dir = new File(overridenPath);
        } else {
            dir = new File(System.getProperty("java.io.tmpdir"), "wagon-git-" + hashPath(remote));
        }
        dir.mkdirs();
        return dir;
    }

Copy link
Owner

Choose a reason for hiding this comment

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

This new feature of providing a directory, interferes somehow, in a "semantic" way, with the "safe checkout" flag (http://synergian.github.io/wagon-git/troubleshooting.html), which wouldn't have any sense any more.

I don't like to have features that interfere one with each other. That's what makes me unhappy :)

I think it's much more elegant to provide a path, like you did. But I would like to remove the old flag so the features don't overlap. And also, as I spent quite some time writing the documentation, I'd like to have it updated to these changes.

I'm not asking you to make those changes, I can. Sadly, I don't have the time right now, so they would have to wait.

Copy link
Author

Choose a reason for hiding this comment

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

Oh I forgot about this flag, you're totally right.
Don't worry, i will do the changes. Also, I was going to update the documentation as well to reflect all the changes. I want to be sure that we agree on the code before getting to deep into documentation =)

Copy link
Author

Choose a reason for hiding this comment

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

So, about this flag, should we keep it and enable it only when wagon.git.permanent.root is not set ? (ie. when wagon.git.permanent.root is set, wagon.git.safe.checkout is considered as false).

Copy link
Owner

Choose a reason for hiding this comment

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

I'm sorry. I'm messing things up. I wrote that part of the code like 4 or 5 years ago. Forget that. Although, the name of the property should be wagon.git.cleanup.directory instead of wagon.git.safe.checkout. I probably changed that at some point. I honestly don't remember.

Change that if you want. I'm okay if you don't.

I'll just await for the documentation of the two new flags.

} else {
dir = new File(System.getProperty("java.io.tmpdir"), "wagon-git-" + hashPath(path));
}
dir.mkdirs();

return dir;
Expand Down Expand Up @@ -55,8 +59,10 @@ private static String hashPath(String path) throws GitException {
}

public static boolean getBooleanEnvironmentProperty(String key) {

return Boolean.parseBoolean(System.getProperty(key, "false"));
}

public static String getStringEnvironmentProperty(String key) {
return System.getProperty(key, null);
}
}