Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Clone #778

Merged
merged 69 commits into from
@ben

Libgit2 and clone need each other. Let's make it happen.

@travisbot

This pull request fails (merged 82e5623e into eaf18ac).

src/clone.c
((92 lines not shown))
+ if (git_path_exists(fullpath)) {
+ giterr_set(GITERR_INVALID, "Destination already exists: %s", fullpath);
+ return GIT_ERROR;
+ }
+
+ return clone_internal(out, origin_url, fullpath, stats, 1);
+}
+
+
+int git_clone(git_repository **out,
+ const char *origin_url,
+ const char *workdir_path,
+ git_indexer_stats *stats)
+{
+ int retcode = GIT_ERROR;
+ char fullpath[512] = {0};
@vmg Owner
vmg added a note

We don't use buffers on the stack for paths. That's why @arrbee implemented the git_buf path helpers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisbot

This pull request fails (merged 09866b6a into eaf18ac).

@travisbot

This pull request fails (merged 9357d626 into eaf18ac).

src/clone.c
((94 lines not shown))
+ return GIT_ERROR;
+ }
+
+ return clone_internal(out, origin_url, fullpath, stats, 1);
+}
+
+
+int git_clone(git_repository **out,
+ const char *origin_url,
+ const char *workdir_path,
+ git_indexer_stats *stats)
+{
+ int retcode = GIT_ERROR;
+ char fullpath[512] = {0};
+
+ p_realpath(workdir_path, fullpath);
@arrbee Owner
arrbee added a note

Try git_path_prettify for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/clone.c
((16 lines not shown))
+#include "fileops.h"
+// TODO #include "checkout.h"
+
+GIT_BEGIN_DECL
+
+
+static int git_checkout_branch(git_repository *repo, const char *branchname)
+{
+ /* TODO */
+ return 0;
+}
+
+/*
+ * submodules?
+ * filemodes?
+ */
@arrbee Owner
arrbee added a note

Also: crlf smudging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisbot

This pull request passes (merged f81e3993 into eaf18ac).

@ben

There are a few notable things these calls don't:

  1. Mirroring (--mirror)
  2. Alternate names for the origin remote (-o)
  3. Support for objects/info/alternates (--shared and --reference)
  4. Overall progress; currently the git_indexer_stats will reach 100% when the network transfer is done, but before the checkout begins.
  5. Depth control or restricting to the history of a single branch (--depth and --single-branch)
  6. Updating of submodules (--recursive)

How important are they for this API? I ask not because I'm planning on implementing them, but because they probably affect the API. I don't want to break the function signatures later on if we decide one of these is critical.

@travisbot

This pull request passes (merged 1e833483 into eaf18ac).

Ben Straub added some commits
Ben Straub Add git_clone and git_clone_bare.
So far they only create a repo, setup the "origin"
remote, and fetch. The API probably needs work as
well; there's no way to get progress information
at this point.

Also uncovered a shortcoming; git_remote_download
doesn't fetch over local transport.
764df57
Ben Straub Add progress reporting to clone. bb1f608
Ben Straub Clone: restructure. f2a855d
Ben Straub Disable failing test (for now). 3c4b008
Ben Straub Disable long-running test. da73fb7
Ben Straub Clone: remove fragile path-handling code.
Also standardized on 3-space indentation. Sorry
about that.
8340dd5
Ben Straub Clone: local branch for remote HEAD.
Now creating a local branch that tracks to the
origin's HEAD branch, and setting HEAD to that.
4fbc899
Ben Straub Clone: prefer "master" as default branch. af58ec9
@travisbot

This pull request passes (merged af58ec9 into 9311423).

src/clone.c
((61 lines not shown))
+
+ git_object_free(head_obj);
+ return retcode;
+}
+
+static int reference_matches_remote_head(const char *head_name, void *payload)
+{
+ struct HeadInfo *head_info = (struct HeadInfo *)payload;
+ git_oid oid;
+
+ /* Stop looking if we've already found a match */
+ if (git_buf_len(&head_info->branchname) > 0) return 0;
+
+ if (!git_reference_name_to_oid(&oid, head_info->repo, head_name) &&
+ !git_oid_cmp(&head_info->remote_head_oid, &oid)) {
+ /* strlen("refs/remotes/origin/") == 20 */
@vmg Owner
vmg added a note

Just add the strlen explicitly and drop the comment. GCC will optimize it into a constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisbot

This pull request fails (merged 14741d6 into 9311423).

@scunz

git.git allows the destination directory to be existing, but mandates it to be empty - shouldn't libgit2 follow that?

sacu@mephista ~ $ mkdir foobar
sacu@mephista ~ $ git clone https://github.com/libgit2/libgit2 foobar
Cloning into 'foobar'...
remote: Counting objects: 18394, done.
remote: Compressing objects: 100% (5383/5383), done.
remote: Total 18394 (delta 13480), reused 17442 (delta 12651)
Receiving objects: 100% (18394/18394), 5.64 MiB | 93 KiB/s, done.
Resolving deltas: 100% (13480/13480), done.
sacu@mephista ~ $ git clone https://github.com/libgit2/libgit2 foobar
fatal: destination path 'foobar' already exists and is not an empty directory.
sacu@mephista ~ $ 

That's a good point, I'll put it on my list.

@travisbot

This pull request passes (merged cb2dc0b into 9311423).

@travisbot

This pull request passes (merged 830388a into 9311423).

Ben Straub added some commits
Ben Straub Checkout: read blob objects to file.
Properly handling file modes. Still needs line-
ending transformations.
24b0d3d
Ben Straub Clone: update index to HEAD.
git_clone now produces a repo that 
`git status` reports as clean!
2b63db4
@travisbot

This pull request passes (merged 2b63db4 into 9311423).

src/checkout.c
((123 lines not shown))
+{
+ int retcode = GIT_ERROR;
+ git_indexer_stats dummy_stats;
+ git_tree *tree;
+ tree_walk_data payload;
+
+ assert(repo);
+ if (!stats) stats = &dummy_stats;
+
+ stats->total = stats->processed = 0;
+ payload.stats = stats;
+ payload.repo = repo;
+
+ if (!get_head_tree(&tree, repo)) {
+ /* Count all the tree nodes for progress information */
+ if (!git_tree_walk(tree, count_walker, GIT_TREEWALK_POST, &payload)) {
@vmg Owner
vmg added a note

This is... heuh... I'm not too sure of this. @peff, does core Git do the walk twice?

@peff Owner
peff added a note

No, definitely not. I believe we actually do the checkout into the in-memory index, then show progress information while actually checking files out to the working tree. So the progress you see in git only starts after we are done looking at the trees entirely.

@ben
ben added a note

I removed the double walk, but now there's no data for payload.stats->total. I'll see what I can do with git_index_read_tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nulltoken nulltoken referenced this pull request in libgit2/libgit2sharp
Closed

Proper pull, fetch, merge support #65

@travisbot

This pull request passes (merged f2d42ee into 9311423).

Ben Straub added some commits
@travisbot

This pull request passes (merged ea81786 into 9311423).

Ben Straub added some commits
Ben Straub Plug leak. 8fb5e40
Ben Straub Reindent. 1c7eb97
include/git2/checkout.h
((19 lines not shown))
+ * @ingroup Git
+ * @{
+ */
+GIT_BEGIN_DECL
+
+/**
+ * Updates files in the working tree to match the version in the index
+ * or HEAD.
+ *
+ * @param repo repository to check out (must be non-bare)
+ * @param origin_url repository to clone from
+ * @param workdir_path local directory to clone to
+ * @param stats pointer to structure that receives progress information (may be NULL)
+ * @return 0 on success, GIT_ERROR otherwise (use git_error_last for information about the error)
+ */
+GIT_EXTERN(int) git_checkout_force(git_repository *repo, git_indexer_stats *stats);
@arrbee Owner
arrbee added a note

I'm guessing you are aware that the comment and the actual prototype do not agree? I was just starting to read this all over to see how things we're going... Are things in a state where they are worth going through?

@ben
ben added a note

I'm always open for review. I wouldn't have opened a PR if I hadn't put on my asbestos underpants.

I was rewriting the summary, and now I'm not sure if this does the right thing. Currently, it updates the workdir to match what's in the index, but maybe that's not simple enough. I'm wondering if it should be git_checkout_to_ref and git_checkout_to_commit. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/checkout.c
((11 lines not shown))
+#include "git2/repository.h"
+#include "git2/refs.h"
+#include "git2/tree.h"
+#include "git2/commit.h"
+#include "git2/blob.h"
+
+#include "common.h"
+#include "refs.h"
+#include "buffer.h"
+#include "repository.h"
+#include "filter.h"
+
+GIT_BEGIN_DECL
+
+
+static int get_head_tree(git_tree **out, git_repository *repo)
@arrbee Owner
arrbee added a note

I feel like we talked about this, but I can't find a record of it. Maybe it wasn't on this PR. Anyhow, there is now a git_repository_head_tree function with exactly the same prototype as this in repository.c that can replace this function. I suspect it wasn't there yet when this code was written.

@ben
ben added a note

I remember that too. I must have rebased it in when I wasn't paying attention. Anyway, it's fixed now. I :purple_heart: deleting code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/clone.c
((181 lines not shown))
+ /* Point HEAD to the same ref as the remote's head */
+ if (!update_head_to_remote(repo, origin)) {
+ retcode = 0;
+ }
+ }
+ }
+ git_remote_disconnect(origin);
+ }
+ git_remote_free(origin);
+ }
+
+ return retcode;
+}
+
+
+static bool is_dot_or_dotdot(const char *name)
@arrbee Owner
arrbee added a note

This is exactly the same as the code in path.c (line 492). Can we rename it git_path_is_dot_or_dotdot and make it an inline function in path.h and share the code in both places?

@ben
ben added a note

Good catch. Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/clone.c
((189 lines not shown))
+ git_remote_free(origin);
+ }
+
+ return retcode;
+}
+
+
+static bool is_dot_or_dotdot(const char *name)
+{
+ return (name[0] == '.' &&
+ (name[1] == '\0' ||
+ (name[1] == '.' && name[2] == '\0')));
+}
+
+/* TODO: p_opendir, p_closedir */
+static bool path_is_okay(const char *path)
@arrbee Owner
arrbee added a note

I think I'd prefer to see a new bool git_path_isdir_and_empty(const char *path) function with a Windows implementation and a non-Windows implementation (less interleaving of #ifdefs would improve readability, even at the cost of a small amount of repeated code).

Then this function could just become a relatively simple !git_path_exists || git_path_isdir_and_empty.

What do you think?

@ben
ben added a note

I like it. :ok_hand:

@ben
ben added a note

Aaaaaand done. git_path_is_empty_dir, at your service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/checkout.c
((41 lines not shown))
+ git_object_free(commit);
+ }
+ git_reference_free(head);
+ }
+
+ return retcode;
+}
+
+typedef struct tree_walk_data
+{
+ git_indexer_stats *stats;
+ git_repository *repo;
+} tree_walk_data;
+
+
+static int apply_filters(git_buf *out,
@arrbee Owner
arrbee added a note

For the purposes of diff, I am going to need a function that easily gets me the in-memory contents of a blob from the ODB with the filters applied. Is there a chance you could make a non-static function out of this and move it somewhere we can share it.

I think what I'd really like is something relatively high-level, a la the blob_contents_to_file but that gives me the data in-memory, preferably in a git_buf. If we had that function, then I think the blob_contents_to_file routine could become much smaller (and this apply_filters function would just be absorbed.

I'm not sure where the right place for such a function would be; perhaps in filter.h/c as a higher-level interface to the filtering functionality. Open to alternative suggestions if you think of a cleaner/more appropriate place.

If you don't want to get into the now, I can tackle it when I get around to crlf handling for diff. Just let me know...

@ben
ben added a note

Something like this? It's a little clunky, since you give it both a blob OID and a path, but both are (unfortunately) necessary.

@arrbee Owner
arrbee added a note

That looks about right. There is no avoiding having to give both the OID and the path since you need the path to check filter application rules.

@ben
ben added a note

Great! I'll move it somewhere accessible for you. I think the filter API is the only good place for it.

@ben
ben added a note

git_filter_blob_contents at your service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@arrbee arrbee commented on the diff
src/checkout.c
((122 lines not shown))
+ break;
+
+ case GIT_OBJ_BLOB:
+ {
+ git_buf fnbuf = GIT_BUF_INIT;
+ git_buf_join_n(&fnbuf, '/', 3,
+ git_repository_workdir(data->repo),
+ path,
+ git_tree_entry_name(entry));
+ retcode = blob_contents_to_file(data->repo, &fnbuf, git_tree_entry_id(entry), attr);
+ git_buf_free(&fnbuf);
+ }
+ break;
+
+ default:
+ retcode = -1;
@arrbee Owner
arrbee added a note

(As you probably know,) this switch is missing symlinks and submodules that should ultimately be supported in checkout. It's probably reasonable to skip both in the initial implementation, but you might consider putting placeholders in as a reminder that they ought to be dealt with, It will make it easier to me to hook in the submodule functionality that I've got in-progress into this code as needed. Also, the symlinks functionality is relatively easy - might be a good starter project for a new hire (or you can just add it when you get a moment).

@ben
ben added a note

As of 09a0399, symlinks are (I think) done. Is there an easy way to identify a submodule node so I can put a better TODO comment?

@scunz
scunz added a note

On an initialized and uptodate submodule, i get:

D:\mgv\src>git ls-tree HEAD | grep libgit2
160000 commit 111ee3fe2d4c6de6729b94235c709986b4079c4b  libgit2

Is attr == 0160000 sufficient?
The SHA1 (111ee in this case) refers to a commit in the submodule.

Edit I just cloned that repository locally - and the un-initialized submodule looks the same.

@arrbee Owner
arrbee added a note

Yes, a submodule will appear as a GIT_OBJ_COMMIT in the tree. The behavior of core git when doing a clone of a submodule is just to create an empty directory at that path. The user is then required to do a git submodule init and git submodule update to correctly populate the empty directory with the submodule content. (There is a --recurse-submodules option to clone, but that will be supported in libgit2 by secondary calls to the submodules, fetch, and clone APIs, I think.)

@ben
ben added a note

Got it. As of dc03369, checkout creates empty dirs for every GIT_OBJ_COMMIT it finds as part of a checkout. That oughtta get you started; let me know if it isn't right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Ben Straub added some commits
Ben Straub Remove duplicate of git_repository_head_tree. 822d9dd
Ben Straub Add git_path_is_dot_or_dotdot.
Also, remove some duplication in the clone test
suite.
c3b5099
Ben Straub Add git_path_is_empty_dir. d024419
@travisbot

This pull request passes (merged c3b5099 into 9311423).

Ben Straub Fix compile and workings on msvc.
Signed-off-by: Ben Straub <bstraub@github.com>
8116738
@travisbot

This pull request passes (merged 8116738 into 9311423).

@travisbot

This pull request passes (merged 71bc89b into 9311423).

src/checkout.c
((19 lines not shown))
+#include "buffer.h"
+#include "repository.h"
+#include "filter.h"
+
+GIT_BEGIN_DECL
+
+
+typedef struct tree_walk_data
+{
+ git_indexer_stats *stats;
+ git_repository *repo;
+ git_odb *odb;
+} tree_walk_data;
+
+
+static int unfiltered_blob_contents(git_buf *out, git_repository *repo, const git_oid *blob_id)
@arrbee Owner
arrbee added a note

In src/blob.h there is already a git_blob__getbuf routine that sets a buffer to the content of a blob. Given that, I think unfiltered_blob_contents can read:

    int error;
    git_blob *blob = NULL;

    if (!(error = git_blob_lookup(&blob, repo, blob_id)))
        error = git_blob__getbuf(out, blob);

    git_blob_free(blob);

    return error;

which feels a bit easier to read to me (as well as propagating the actual error from git_blob_lookup if there is a failure instead of always converting it to a generic -1 value).

@ben
ben added a note

Love it. Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/checkout.c
((44 lines not shown))
+ git_blob_free(blob);
+ retcode = 0;
+ }
+ return retcode;
+}
+
+static int filtered_blob_contents(git_buf *out, git_repository *repo, const git_oid *oid, const char *path)
+{
+ int retcode = GIT_ERROR;
+
+ git_buf unfiltered = GIT_BUF_INIT;
+ if (!unfiltered_blob_contents(&unfiltered, repo, oid)) {
+ git_vector filters = GIT_VECTOR_INIT;
+ int filter_count = git_filters_load(&filters, repo,
+ path, GIT_FILTER_TO_WORKTREE);
+ if (filter_count >= 0) {
@arrbee Owner
arrbee added a note

This block of code seems like a waste to me. git_filters_apply should just copy the input to the output if the filter_count is zero and you should not have to special case that situation. If that is not the behavior of git_filters_apply then let's fix it instead of requiring 10 lines of code just to load and apply an array of filters.

@ben
ben added a note

I didn't totally grok the way git_filters_apply works on my first read. I guess I was just visually grepping for some kind of copy. Anyways, you're completely correct, I've removed the offending lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/checkout.c
((91 lines not shown))
+ }
+ }
+ git_buf_free(&filteredcontents);
+
+ return retcode;
+}
+
+static int checkout_walker(const char *path, git_tree_entry *entry, void *payload)
+{
+ int retcode = 0;
+ tree_walk_data *data = (tree_walk_data*)payload;
+ int attr = git_tree_entry_attributes(entry);
+
+ /* TODO: handle submodules */
+
+ if (S_ISLNK(attr)) {
@arrbee Owner
arrbee added a note

This is a pretty minor detail, I realize, but in my mind, I would move the switch on the git_tree_entry_type to the outer scope and only check for the S_ISLNK for GIT_OBJ_BLOB since that is the object type under which a link will be stored.

@ben
ben added a note

I'll do that as part of finishing the handling of links.

@ben
ben added a note

How does this look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
include/git2/checkout.h
((16 lines not shown))
+ * @file git2/checkout.h
+ * @brief Git checkout routines
+ * @defgroup git_checkout Git checkout routines
+ * @ingroup Git
+ * @{
+ */
+GIT_BEGIN_DECL
+
+/**
+ * Updates files in the working tree to match the version in the index.
+ *
+ * @param repo repository to check out (must be non-bare)
+ * @param stats pointer to structure that receives progress information (may be NULL)
+ * @return 0 on success, GIT_ERROR otherwise (use git_error_last for information about the error)
+ */
+GIT_EXTERN(int) git_checkout_force(git_repository *repo, git_indexer_stats *stats);
@arrbee Owner
arrbee added a note

So, the behavior as implemented appears to be to call creat() on each file in the head tree. It would probably be nice to let the user specify options that chose between creat (which is actually O_CREAT | O_TRUNC | O_WRONLY) and exclusive overwrite (i.e. O_CREAT | O_TRUNC | O_WRONLY | O_EXCL).

I'm curious to hear @tanoku's opinion also, but I wonder if we might want to allow other parameters, such as: bypass filters, checkout to something other than workdir, checkout something other than HEAD, etc. If we were to add these things, you'd just pass an options structure in where NULLs would set the various options to their defaults. I just don't know if anyone would use these options...

@ben
ben added a note

@nulltoken, @tanoku, @arrbee, how would you feel about something like this?

typedef struct git_checkout_opts {
    git_indexer_stats stats;
    int existing_file_action;
    int apply_filters;
    int dir_mode;
    int file_open_mode;
} git_checkout_opts;

#define GIT_CHECKOUT_DEFAULT_OPTS {  \
    {0},                              \
    GIT_CHECKOUT_OVERWRITE_EXISTING,  \
    true,                             \
    GIT_DIR_MODE,                     \
    O_CREAT|O_TRUNC|O_WRONLY          \
}

This means the API surface would look more like this:

int git_checkout_index(git_repository *repo, git_checkout_opts *opts);
int git_checkout_head(git_repository *repo, git_checkout_opts *opts);
int git_checkout_ref(git_reference *ref, git_checkout_opts *opts);
// ...

... which seems more logical to me.

@vmg Owner
vmg added a note

The define with the defaults makes my spider sense tingle. Instead: set the default automatically when any of the options is 0. Also, the indexer stats should be outside of the options struct so they can always be made optional. :+1:

int git_checkout_index(git_repository *repo, git_checkout_opts *opts, git_indexer_stats *stats);
@peff Owner
peff added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Ben Straub added some commits
Ben Straub Use git_blob__getbuf. 41ad70d
Ben Straub Migrate code to git_filter_blob_contents.
Also removes the unnecessary check for filter 
length, since git_filters_apply does the right 
thing when there are none, and it's more efficient
than this.
9587895
Ben Straub Checkout: handle symlinks.
Includes unfinished win32 implementation.
1d68fcd
@travisbot

This pull request fails (merged 1d68fcd into 9311423).

@travisbot

This pull request passes (merged 3e026f1 into ea5d2ce).

@travisbot

This pull request passes (merged 09a0399 into ea5d2ce).

@travisbot

This pull request passes (merged dc03369 into ea5d2ce).

Ben Straub added some commits
Ben Straub checkout: introduce git_checkout_opts
Refactor checkout into several more-sensible
entry points, which consolidates common options
into a single structure that may be passed around.
ef9905c
Ben Straub Restructure for better checkout options
* Removed the #define for defaults
* Promoted progress structure to top-level API call
  argument
b401bac
Ben Straub Checkout: implementation of most options 095ccc0
@travisbot

This pull request fails (merged 095ccc0 into ea5d2ce).

@travisbot

This pull request fails (merged 6eb240b into ea5d2ce).

@travisbot

This pull request fails (merged 15445f9 into ea5d2ce).

Ben Straub added some commits
@travisbot

This pull request fails (merged 4d83399 into 31637cd).

Ben Straub added some commits
Ben Straub Checkout: add head- and ref-centric checkouts.
Renamed git_checkout_index to what it really was,
and removed duplicate code from clone.c. Added
git_checkout_ref, which updates HEAD and hands off
to git_checkout_head.

Added tests for the options the caller can pass to
git_checkout_*.
b31667f
Ben Straub Fix testrepo ref count to include new branch. 32beb2e
Ben Straub Checkout: disable file-mode test on win32. e0681f6
@travisbot

This pull request fails (merged e0681f6 into 31637cd).

@ben

@tanoku @arrbee How do you like this? Checkout calls now all take an opts structure and a git_indexer_status to fill in for progress (though only half works so far).

Ben Straub added some commits
Ben Straub Checkout: use git_index_read_tree_with_stats.
New variant of git_index_read_tree that fills in
the 'total' field of a git_indexer_stats struct
as it's walking the tree.
f1587b9
Ben Straub Add clone to the network example. 84595a3
@travisbot

This pull request fails (merged 84595a3 into 31637cd).

@travisbot

This pull request fails (merged 1ab835b0 into 31637cd).

Ben Straub Enable stats on git_index_read_tree.
Replace with the contents of 
git_index_read_tree_with_stats() and improve
documentation comments.
4bf5115
@travisbot

This pull request fails (merged 4bf5115 into 50364dd).

@travisbot

This pull request fails (merged 383fb79 into 50364dd).

@travisbot

This pull request passes (merged 3f584b5 into 50364dd).

Ben Straub Checkout: handle file modes properly.
Global file mode override now works properly with
the file mode stored in the tree node.
8e4aae1
@travisbot

This pull request passes (merged 8e4aae1 into 50364dd).

include/git2/clone.h
((18 lines not shown))
+ * @brief Git cloning routines
+ * @defgroup git_clone Git cloning routines
+ * @ingroup Git
+ * @{
+ */
+GIT_BEGIN_DECL
+
+/**
+ * TODO
+ *
+ * @param out pointer that will receive the resulting repository object
+ * @param origin_url repository to clone from
+ * @param workdir_path local directory to clone to
+ * @param fetch_stats pointer to structure that receives fetch progress information (may be NULL)
+ * @param checkout_opts options for the checkout step (may be NULL)
+ * @return 0 on success, GIT_ERROR otherwise (use git_error_last for information about the error)
@carlosmn Owner

The function is giterr_last() nowadays.

@ben
ben added a note

That would be my getting used to vim and ^N. Fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/checkout.c
((207 lines not shown))
+ return retcode;
+}
+
+
+int git_checkout_reference(git_reference *ref,
+ git_checkout_opts *opts,
+ git_indexer_stats *stats)
+{
+ git_repository *repo= git_reference_owner(ref);
+ git_reference *head = NULL;
+ int retcode = GIT_ERROR;
+
+ if ((retcode = git_reference_lookup(&head, repo, GIT_HEAD_FILE)) < 0)
+ return retcode;
+
+ if ((retcode = git_reference_set_target(head, git_reference_name(ref))) < 0)
@carlosmn Owner
carlosmn added a note

This will fail if HEAD is detached. Something like git_reference_create_symbolic(&head, repo, GIT_HEAD_FILE, git_reference_name(ref), 1); would work in any case.

@ben
ben added a note

NICE CATCH. I did it your way, and verified it using a debugger breakpoint. I'll write an automated test when git_checkout_commit comes along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosmn carlosmn commented on the diff
src/clone.c
((16 lines not shown))
+#include "git2/revparse.h"
+#include "git2/branch.h"
+#include "git2/config.h"
+#include "git2/checkout.h"
+#include "git2/commit.h"
+#include "git2/tree.h"
+
+#include "common.h"
+#include "remote.h"
+#include "fileops.h"
+#include "refs.h"
+#include "path.h"
+
+GIT_BEGIN_DECL
+
+struct HeadInfo {
@carlosmn Owner
carlosmn added a note

CamelCase makes me sad :disappointed:

@ben
ben added a note

Changed it to head_info. :smirk:? :relaxed:? :bowtie:?

@carlosmn Owner
carlosmn added a note

:smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosmn
Owner

Using git_indexer_stats is fine for now, but that's going to grow one or two fields really are indexer-specific, so we might want to add a new struct that has current/total (which would be better names anyway).

The amount of downloaded data should be exposed to the user so they can calculate download speed, but that's not so urgent. The network code is going to learn about sidebands which means progress output from the server. Adding a git_remote_callbacks parameter to clone would be good.

Ben Straub added some commits
Ben Straub Add checkout.h to git2.h.
Also correcting some documentation strings.
5280f4e
Ben Straub Checkout: fix problem with detached HEAD. 5f4d2f9
@ben
ben commented

@carlosmn Those are both great ideas, and they're going on my list (along with git_checkout_commit and friends). I'll tackle them with their own PRs, though; this one is big enough.

@travisbot

This pull request passes (merged 8b67f72 into 50364dd).

@carlosmn
Owner

Sure, having PRs for those things is cool, keeps thing simpler.

@scunz scunz commented on the diff
include/git2/clone.h
((21 lines not shown))
+ * @{
+ */
+GIT_BEGIN_DECL
+
+/**
+ * Clone a remote repository, and checkout the branch pointed to by the remote
+ * HEAD.
+ *
+ * @param out pointer that will receive the resulting repository object
+ * @param origin_url repository to clone from
+ * @param workdir_path local directory to clone to
+ * @param fetch_stats pointer to structure that receives fetch progress information (may be NULL)
+ * @param checkout_opts options for the checkout step (may be NULL)
+ * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information about the error)
+ */
+GIT_EXTERN(int) git_clone(git_repository **out,
@scunz
scunz added a note

As git clone also has a big bunch of command line options, do you plan on something like arrbee's git_repository_init_ext (#844)?

@ben
ben added a note

It looks like a pretty good fit, yeah. It's on my list to shrink the clone code by using that once they're both merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisbot

This pull request passes (merged aa549d3 into 50364dd).

@travisbot

This pull request fails (merged eb87800 into 50364dd).

@ben

@Haacked @aroben About the x bit on Windows. Here's my first stab at this (pretty sure it went in my eye). It's a good first step, but I have questions.

Properties

  1. What's the ideal result from:
    1. the u+x bit?
    2. the g+x bit?
    3. the o+x bit?
  2. Should we go through all the SID/DACL stuff for chmod also, or is the behavior of _wchmod good enough?
  3. It may turn out that some users don't have the ability to do this, but in that case it should probably fail silently, since there's nothing we can do about it.

OR

We could ship this as-is. It's closer to x bit support than msysgit has.

@aroben
Owner
  1. I propose:
    1. u+x: Should grant the file's owner (GetSecurityDescriptorOwner) the Read & Execute permission
    2. g+x: Should grant the file's owner (GetSecurityDescriptorGroup) the Read & Execute permission
    3. o+x: Should grant the EVERYONE group the Read & Execute permission
  2. _wchmod is not good enough. It doesn't seem to actually modify whether the file can be executed. (This is what git core does today, and it's not working.)
  3. Agreed.
@Haacked
Owner

I must admit my ignorance. What does u+x etc. mean?

@aroben Your #1 and #2 are the same. Is that intentional?

@arrbee
Owner

@haacked On Unix systems, there are 3 bits each for 3 categories of folks that might access a file. The bits are r = read, w = write, and x = execute. The user classes are u = user (i.e. the owner of the file), g = group (i.e. a unix group id associated with the file), and o = other (i.e. everyone else). So, u+x means the owner of the file has the execute bit set, etc. On Windows, there is not really an equivalent, which is the problem.

@aroben @benstraub Inside a git repository, there are only 0644 and 0755 modes - there are no other permissions on blobs, so we are really just talking about one thing to do for blobs with the execute permission and one thing to do for blobs without it. We don't actually have to distinguish between u, g, and o permissions.

@nulltoken
Collaborator
@nulltoken
Collaborator

Found it. http://stackoverflow.com/a/8347325/335418

Haven't had the curiosity to look at the source though, so this might be completely irrelevant :-/

@arrbee
Owner

@nulltoken Interesting! The only time that I know of 0664 coming into play is in the .git directory of a repository where --shared=group or --shared=everybody and that is not for checked out files, just for .git.

Oh, I see the core git code now. It is commented with:

        /*
         * This is nonstandard, but we had a few of these
         * early on when we honored the full set of mode
         * bits..
         */
        case S_IFREG | 0664:
            if (!strict)
                break;

Non-strict fsck will allow it because it used to be possible, but modern git won't and strict fsck will warn about a bad mode.

@vmg
Owner

I don't feel like we should support these non-standard file modes. I wonder how many repositories would we break if we dropped them altogether...

@arrbee
Owner

There is definitely no need to support these modes. From what I can tell, core git only "supports" them in that fsck won't barf if it sees them, but it certainly doesn't make any effort to accept them as input or generate them as output.

WRT this PR, I think the only question is: when creating a blob that has 0755 permission, should be make an extra effort on Windows to grant the owner of the file Read & Execute permission, even though the code to do that is a bit ugly. I think it would be nice feature and it sounds like @aroben does too. It also sounds like the _wchmod behavior does not do that, so we would have to go a bit further to get the right functionality.

@aroben
Owner

Oops, I meant to say:

2 g+x: Should grant the file's group (GetSecurityDescriptorGroup) the Read & Execute permission

@peff
Owner
@nulltoken
Collaborator

@peff Thanks for the answer.

Even if slightly off-topic, one last question: when browsing a Tree, and encountering a Blob which mode is 100664, should it be now displayed/listed as 100664 or as 100644?

@peff
Owner

Git-core will silently convert it to 100644. Not explicitly, but by virtue of the fact that we simply don't bother looking at the group bit. I'm not sure how common this actually is, though. You can usually find these odd bits of history in git.git or linux-2.6.git, since they were the early adopters, but neither of them seems to have such a tree.

@vmg vmg merged commit f98c32f into libgit2:development

1 check failed

Details default The Travis build failed
@vmg
Owner

:sparkles:

@ben ben referenced this pull request in libgit2/node-gitteh
Open

Gitteh needs a primary maintainer! #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 21, 2012
  1. Add git_clone and git_clone_bare.

    Ben Straub authored
    So far they only create a repo, setup the "origin"
    remote, and fetch. The API probably needs work as
    well; there's no way to get progress information
    at this point.
    
    Also uncovered a shortcoming; git_remote_download
    doesn't fetch over local transport.
  2. Add progress reporting to clone.

    Ben Straub authored
  3. Clone: restructure.

    Ben Straub authored
  4. Disable failing test (for now).

    Ben Straub authored
  5. Disable long-running test.

    Ben Straub authored
  6. Clone: remove fragile path-handling code.

    Ben Straub authored
    Also standardized on 3-space indentation. Sorry
    about that.
  7. Clone: local branch for remote HEAD.

    Ben Straub authored
    Now creating a local branch that tracks to the
    origin's HEAD branch, and setting HEAD to that.
  8. Clone: prefer "master" as default branch.

    Ben Straub authored
  9. Clone: minor cleanup and whitespace.

    Ben Straub authored
  10. Clone: new home for git_checkout_force.

    Ben Straub authored
  11. Clone: replace one hardcoded value with another.

    Ben Straub authored
Commits on Jun 22, 2012
  1. Checkout: initial tree walkers.

    Ben Straub authored
  2. Fix warning on msvc build.

    Ben Straub authored
  3. Clone: allow empty dirs.

    Ben Straub authored
  4. Clone: non-empty-dir test, now for Win32.

    Ben Straub authored
Commits on Jun 26, 2012
  1. Checkout: read blob objects to file.

    Ben Straub authored
    Properly handling file modes. Still needs line-
    ending transformations.
  2. Clone: update index to HEAD.

    Ben Straub authored
    git_clone now produces a repo that 
    `git status` reports as clean!
Commits on Jul 6, 2012
  1. Apply filters on checkout.

    Ben Straub authored
Commits on Jul 10, 2012
  1. Checkout: reindent, fix uninit. variable.

    Ben Straub authored
  2. Checkout: add structure for CRLF.

    Ben Straub authored
  3. Checkout: only walk tree once while checking out.

    Ben Straub authored
  4. Tabify.

    Ben Straub authored
  5. Plug leak.

    Ben Straub authored
  6. Reindent.

    Ben Straub authored
Commits on Jul 11, 2012
  1. Remove duplicate of git_repository_head_tree.

    Ben Straub authored
  2. Add git_path_is_dot_or_dotdot.

    Ben Straub authored
    Also, remove some duplication in the clone test
    suite.
  3. Add git_path_is_empty_dir.

    Ben Straub authored
Commits on Jul 12, 2012
  1. Fix compile and workings on msvc.

    Ben Straub authored
    Signed-off-by: Ben Straub <bstraub@github.com>
  2. Move is_dot_or_dotdotW into path.h.

    Ben Straub authored
Commits on Jul 14, 2012
  1. Fix documentation comment to match actual params.

    Ben Straub authored
  2. Add checkout test suite.

    Ben Straub authored
    Removed 'bare' option from test repository to 
    allow checkout tests.
  3. Create filtered_blob_contents out of parts on hand.

    Ben Straub authored
  4. Disable test that aren't quite ready yet.

    Ben Straub authored
Commits on Jul 16, 2012
  1. Use git_blob__getbuf.

    Ben Straub authored
  2. Migrate code to git_filter_blob_contents.

    Ben Straub authored
    Also removes the unnecessary check for filter 
    length, since git_filters_apply does the right 
    thing when there are none, and it's more efficient
    than this.
Commits on Jul 17, 2012
  1. Checkout: handle symlinks.

    Ben Straub authored
    Includes unfinished win32 implementation.
  2. Merge branch 'development' into clone

    Ben Straub authored
  3. Update master-tip to fix unit test.

    Ben Straub authored
Commits on Jul 18, 2012
  1. Checkout: obey core.symlinks.

    Ben Straub authored
  2. Checkout: make core.symlinks test work on OSX.

    Ben Straub authored
Commits on Jul 22, 2012
  1. filter: fix memory leak

    Ben Straub authored
  2. checkout: create submodule dirs

    Ben Straub authored
Commits on Jul 26, 2012
  1. checkout: introduce git_checkout_opts

    Ben Straub authored
    Refactor checkout into several more-sensible
    entry points, which consolidates common options
    into a single structure that may be passed around.
  2. Restructure for better checkout options

    Ben Straub authored
    * Removed the #define for defaults
    * Promoted progress structure to top-level API call
      argument
Commits on Jul 27, 2012
  1. Checkout: implementation of most options

    Ben Straub authored
  2. Checkout: use caller's flags for open()

    Ben Straub authored
  3. Turn off network-dependent test for CI.

    Ben Straub authored
  4. Use new git_remote_update_tips signature.

    Ben Straub authored
  5. Fix mismatched git_branch_create args.

    Ben Straub authored
  6. Checkout: handle deeply-nested submodules better.

    Ben Straub authored
    Now creating intermediate directories where the
    submodule is deep, like "src/deps/foosubmodule".
  7. Adjust for msvc pedantry.

    Ben Straub authored
Commits on Jul 28, 2012
  1. Checkout: add head- and ref-centric checkouts.

    Ben Straub authored
    Renamed git_checkout_index to what it really was,
    and removed duplicate code from clone.c. Added
    git_checkout_ref, which updates HEAD and hands off
    to git_checkout_head.
    
    Added tests for the options the caller can pass to
    git_checkout_*.
  2. Fix testrepo ref count to include new branch.

    Ben Straub authored
  3. Checkout: disable file-mode test on win32.

    Ben Straub authored
Commits on Jul 31, 2012
  1. Checkout: use git_index_read_tree_with_stats.

    Ben Straub authored
    New variant of git_index_read_tree that fills in
    the 'total' field of a git_indexer_stats struct
    as it's walking the tree.
  2. Add clone to the network example.

    Ben Straub authored
  3. Enable stats on git_index_read_tree.

    Ben Straub authored
    Replace with the contents of 
    git_index_read_tree_with_stats() and improve
    documentation comments.
  4. Checkout: save index on checkout.

    Ben Straub authored
  5. Rename example function to avoid name collision.

    Ben Straub authored
  6. Try to fix Travis.

    Ben Straub authored
  7. Checkout: handle file modes properly.

    Ben Straub authored
    Global file mode override now works properly with
    the file mode stored in the tree node.
Commits on Aug 1, 2012
  1. Checkout: crlf filter.

    Ben Straub authored
  2. Checkout: fix crlf tests under win32.

    Ben Straub authored
  3. Add checkout.h to git2.h.

    Ben Straub authored
    Also correcting some documentation strings.
  4. Checkout: fix problem with detached HEAD.

    Ben Straub authored
  5. Add documentation for clone methods.

    Ben Straub authored
Commits on Aug 2, 2012
  1. Clean up a TODO comment.

    Ben Straub authored
Commits on Aug 6, 2012
  1. Checkout: fix memory leak in tests.

    Ben Straub authored
Something went wrong with that request. Please try again.