From 4d7905c579a4a89a4894ec1cea1f593338d1042f Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Wed, 25 May 2011 16:04:29 +0200 Subject: [PATCH 1/8] blob: Require stat information for git_blob_create_fromfile() In order to be able to write symlinks with git_blob_create_fromfile(), we need to check whether the file to be written is a symbolic link or not. Since the calling function of git_blob_create_fromfile() is likely to have stated the file before calling, we make it pass the stat. The reason for this is that writing symbolic link blobs is significantly different from writing ordinary files - we do not want to open the link destination but instead want to write the link itself, regardless of whether it exists or not. Previously, index_init_entry() used to error out if the file to be added was a symlink that pointed to a nonexistent file. Fix this behaviour to add the file regardless of whether it exists. This mimics git.git's behaviour. --- include/git2/blob.h | 2 +- src/blob.c | 35 ++++++++++++++++++++++++----------- src/index.c | 2 +- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/include/git2/blob.h b/include/git2/blob.h index e366ce880..1cfff841d 100644 --- a/include/git2/blob.h +++ b/include/git2/blob.h @@ -119,7 +119,7 @@ GIT_EXTERN(int) git_blob_rawsize(git_blob *blob); * relative to the repository's working dir * @return 0 on success; error code otherwise */ -GIT_EXTERN(int) git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path); +GIT_EXTERN(int) git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path, struct stat st); /** diff --git a/src/blob.c b/src/blob.c index 6ab58d6b2..b00fc25af 100644 --- a/src/blob.c +++ b/src/blob.c @@ -78,39 +78,51 @@ int git_blob_create_frombuffer(git_oid *oid, git_repository *repo, const void *b return GIT_SUCCESS; } -int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path) +int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path, struct stat st) { - int error, fd; + int error, islnk; + int fd = 0; char full_path[GIT_PATH_MAX]; char buffer[2048]; git_off_t size; git_odb_stream *stream; + islnk = S_ISLNK(st.st_mode); + if (repo->path_workdir == NULL) return git__throw(GIT_ENOTFOUND, "Failed to create blob. (No working directory found)"); git__joinpath(full_path, repo->path_workdir, path); - if ((fd = gitfo_open(full_path, O_RDONLY)) < 0) - return git__throw(GIT_ENOTFOUND, "Failed to create blob. Could not open '%s'", full_path); + if (!islnk) { + if ((fd = gitfo_open(full_path, O_RDONLY)) < 0) + return git__throw(GIT_ENOTFOUND, "Failed to create blob. Could not open '%s'", full_path); - if ((size = gitfo_size(fd)) < 0 || !git__is_sizet(size)) { - gitfo_close(fd); - return git__throw(GIT_EOSERR, "Failed to create blob. '%s' appears to be corrupted", full_path); + if ((size = gitfo_size(fd)) < 0 || !git__is_sizet(size)) { + gitfo_close(fd); + return git__throw(GIT_EOSERR, "Failed to create blob. '%s' appears to be corrupted", full_path); + } + } else { + size = st.st_size; } if ((error = git_odb_open_wstream(&stream, repo->db, (size_t)size, GIT_OBJ_BLOB)) < GIT_SUCCESS) { - gitfo_close(fd); + if (!islnk) + gitfo_close(fd); return git__rethrow(error, "Failed to create blob"); } while (size > 0) { ssize_t read_len; - read_len = read(fd, buffer, sizeof(buffer)); + if (!islnk) + read_len = read(fd, buffer, sizeof(buffer)); + else + read_len = readlink(full_path, buffer, sizeof(buffer)); if (read_len < 0) { - gitfo_close(fd); + if (!islnk) + gitfo_close(fd); stream->free(stream); return git__throw(GIT_EOSERR, "Failed to create blob. Can't read full file"); } @@ -121,7 +133,8 @@ int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *pat error = stream->finalize_write(oid, stream); stream->free(stream); - gitfo_close(fd); + if (!islnk) + gitfo_close(fd); if (error < GIT_SUCCESS) return git__rethrow(error, "Failed to create blob"); diff --git a/src/index.c b/src/index.c index e25c899a3..6aefe4432 100644 --- a/src/index.c +++ b/src/index.c @@ -425,7 +425,7 @@ static int index_init_entry(git_index_entry *entry, git_index *index, const char entry->file_size = st.st_size; /* write the blob to disk and get the oid */ - if ((error = git_blob_create_fromfile(&entry->oid, index->repository, rel_path)) < GIT_SUCCESS) + if ((error = git_blob_create_fromfile(&entry->oid, index->repository, rel_path, st)) < GIT_SUCCESS) return git__rethrow(error, "Failed to initialize index entry"); entry->flags |= (stage << GIT_IDXENTRY_STAGESHIFT); From 1869b31e0c99831b1de394ef8cb4d0c9ee633bea Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Wed, 25 May 2011 16:11:57 +0200 Subject: [PATCH 2/8] index/fileops: Correctly process symbolic links gitfo_exists() used to error out if the given file was a symbolic link, due to access() returning an error code. This is not expected behaviour, as gitfo_exists() should only check whether the file itself exists, not its link target if it is a symbolic link. Fix this by calling gitfo_lstat() instead, which is just a wrapper for lstat(). Also fix the same error in index_init_entry(). --- src/fileops.c | 4 +++- src/fileops.h | 1 + src/index.c | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/fileops.c b/src/fileops.c index b1a6bb8b1..92b95c5dd 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -172,7 +172,9 @@ int gitfo_isfile(const char *path) int gitfo_exists(const char *path) { assert(path); - return access(path, F_OK); + + struct stat st; + return gitfo_lstat(path, &st); } git_off_t gitfo_size(git_file fd) diff --git a/src/fileops.h b/src/fileops.h index c114508db..f27f2c9e3 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -94,6 +94,7 @@ extern int gitfo_mv_force(const char *from, const char *to); #define gitfo_stat(p,b) stat(p, b) #define gitfo_fstat(f,b) fstat(f, b) +#define gitfo_lstat(p,b) lstat(p,b) #define gitfo_unlink(p) unlink(p) #define gitfo_rmdir(p) rmdir(p) diff --git a/src/index.c b/src/index.c index 6aefe4432..e1b52610b 100644 --- a/src/index.c +++ b/src/index.c @@ -405,7 +405,7 @@ static int index_init_entry(git_index_entry *entry, git_index *index, const char if (gitfo_exists(full_path) < 0) return git__throw(GIT_ENOTFOUND, "Failed to initialize entry. %s does not exist", full_path); - if (gitfo_stat(full_path, &st) < 0) + if (gitfo_lstat(full_path, &st) < 0) return git__throw(GIT_EOSERR, "Failed to initialize entry. %s appears to be corrupted", full_path); if (stage < 0 || stage > 3) From c1a2a14e022caae68112497f5e6862f9b59f1260 Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Wed, 25 May 2011 16:16:41 +0200 Subject: [PATCH 3/8] index: Correctly write entry mode The entry mode flags for an entry created from a path name were not correctly written if the entry was a symlink. The st_mode of a statted symlink is 0120777, however git requires the mode to read 0120000, because it does not care about permissions of symlinks. Introduce index_create_mode() that correctly writes the mode flags in the form expected by git. --- src/index.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/index.c b/src/index.c index e1b52610b..afd9cfccd 100644 --- a/src/index.c +++ b/src/index.c @@ -138,6 +138,15 @@ int unmerged_cmp(const void *a, const void *b) return strcmp(info_a->path, info_b->path); } +unsigned int index_create_mode(unsigned int mode) +{ + if (S_ISLNK(mode)) + return S_IFLNK; + if (S_ISDIR(mode) || (mode & S_IFMT) == 0160000) + return 0160000; + return S_IFREG | ((mode & 0100) ? 0755 : 0644); +} + static int index_initialize(git_index **index_out, git_repository *owner, const char *index_path) { git_index *index; @@ -419,7 +428,7 @@ static int index_init_entry(git_index_entry *entry, git_index *index, const char /* entry.ctime.nanoseconds = st.st_ctimensec; */ entry->dev= st.st_rdev; entry->ino = st.st_ino; - entry->mode = st.st_mode; + entry->mode = index_create_mode(st.st_mode); entry->uid = st.st_uid; entry->gid = st.st_gid; entry->file_size = st.st_size; From cbf4f9f41f18bdb812fb854cd7fd8c4ce62ed44a Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Wed, 25 May 2011 16:31:14 +0200 Subject: [PATCH 4/8] common: Include stat.h in include/git2/common.h instead of src/common.h 00582bcb introduced a change to git_blob_create_fromfile() that required the caller to pass a stat struct. This means that we need to include stat.h higher in the hierarchy of includes. --- include/git2/common.h | 1 + src/common.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/include/git2/common.h b/include/git2/common.h index 9a27ac2e5..d08935f8c 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -28,6 +28,7 @@ #include "thread-utils.h" #include #include +#include #ifdef __cplusplus # define GIT_BEGIN_DECL extern "C" { diff --git a/src/common.h b/src/common.h index f4f11fd2f..fc444c4d4 100644 --- a/src/common.h +++ b/src/common.h @@ -22,7 +22,6 @@ #include #include -#include #ifdef GIT_WIN32 From b74c867a1acfdc5ef0cfa91ba49d3f52d0ac6237 Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Tue, 7 Jun 2011 11:11:09 +0200 Subject: [PATCH 5/8] blob: Stat path inside git_blob_create_fromfile 00582bc introduced a change that required the caller of git_blob_create_fromfile() to pass a struct stat with the stat information for the file. Several developers pointed out that this would make life hard for the bindings developers as struct stat isn't widely supported by other languages. Make git_blob_create_fromfile() stat the path itself, eliminating the need for the file to be stat'ed by the caller. This makes index_init_entry() more costly as the file will be stat'ed twice but makes life easier for everyone else. --- include/git2/blob.h | 2 +- src/blob.c | 5 ++++- src/index.c | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/git2/blob.h b/include/git2/blob.h index 1cfff841d..e366ce880 100644 --- a/include/git2/blob.h +++ b/include/git2/blob.h @@ -119,7 +119,7 @@ GIT_EXTERN(int) git_blob_rawsize(git_blob *blob); * relative to the repository's working dir * @return 0 on success; error code otherwise */ -GIT_EXTERN(int) git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path, struct stat st); +GIT_EXTERN(int) git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path); /** diff --git a/src/blob.c b/src/blob.c index b00fc25af..12f468ef7 100644 --- a/src/blob.c +++ b/src/blob.c @@ -78,7 +78,7 @@ int git_blob_create_frombuffer(git_oid *oid, git_repository *repo, const void *b return GIT_SUCCESS; } -int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path, struct stat st) +int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *path) { int error, islnk; int fd = 0; @@ -86,6 +86,9 @@ int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *pat char buffer[2048]; git_off_t size; git_odb_stream *stream; + struct stat st; + + gitfo_lstat(path, &st); islnk = S_ISLNK(st.st_mode); diff --git a/src/index.c b/src/index.c index afd9cfccd..3d2e72e56 100644 --- a/src/index.c +++ b/src/index.c @@ -434,7 +434,7 @@ static int index_init_entry(git_index_entry *entry, git_index *index, const char entry->file_size = st.st_size; /* write the blob to disk and get the oid */ - if ((error = git_blob_create_fromfile(&entry->oid, index->repository, rel_path, st)) < GIT_SUCCESS) + if ((error = git_blob_create_fromfile(&entry->oid, index->repository, rel_path)) < GIT_SUCCESS) return git__rethrow(error, "Failed to initialize index entry"); entry->flags |= (stage << GIT_IDXENTRY_STAGESHIFT); From ee4912bf795237cb235b3d6533e9046fcd7e12b7 Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Tue, 7 Jun 2011 11:15:23 +0200 Subject: [PATCH 6/8] Revert "common: Include stat.h in include/git2/common.h instead of src/common.h" This reverts commit df1c98ab6d6171ed63729195bd190b54b67fe530. As 8a27b6b reverts the exposition of struct stat to the external API, we do not need - indeed, do not want - struct stat to be in the outer include layer. --- include/git2/common.h | 1 - src/common.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/include/git2/common.h b/include/git2/common.h index d08935f8c..9a27ac2e5 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -28,7 +28,6 @@ #include "thread-utils.h" #include #include -#include #ifdef __cplusplus # define GIT_BEGIN_DECL extern "C" { diff --git a/src/common.h b/src/common.h index fc444c4d4..f4f11fd2f 100644 --- a/src/common.h +++ b/src/common.h @@ -22,6 +22,7 @@ #include #include +#include #ifdef GIT_WIN32 From fdd1e04ce766c40be90a7b07580842a3f8d76b2e Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Tue, 7 Jun 2011 14:10:06 +0200 Subject: [PATCH 7/8] fileops: Allow differentiation between deep and shallow exists() When calling gitfo_exists() on a symbolic link, sometimes we need to simply check whether the link exists and sometimes we need to check whether the file pointed to by the symlink exists. Introduce a new function gitfo_shallow_exists that only checks if the link exists and revert gitfo_exists to the original functionality of checking whether the file pointed to by the link exists. --- src/fileops.c | 6 ++++++ src/index.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/fileops.c b/src/fileops.c index 92b95c5dd..b11119b91 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -170,6 +170,12 @@ int gitfo_isfile(const char *path) } int gitfo_exists(const char *path) +{ + assert(path); + return access(path, F_OK); +} + +int gitfo_shallow_exists(const char *path) { assert(path); diff --git a/src/index.c b/src/index.c index 3d2e72e56..798d9e918 100644 --- a/src/index.c +++ b/src/index.c @@ -411,7 +411,7 @@ static int index_init_entry(git_index_entry *entry, git_index *index, const char git__joinpath(full_path, index->repository->path_workdir, rel_path); - if (gitfo_exists(full_path) < 0) + if (gitfo_shallow_exists(full_path) < 0) return git__throw(GIT_ENOTFOUND, "Failed to initialize entry. %s does not exist", full_path); if (gitfo_lstat(full_path, &st) < 0) From 27a1b382c186dd4267924728344213de2a3692c3 Mon Sep 17 00:00:00 2001 From: Jakob Pfender Date: Tue, 7 Jun 2011 14:15:55 +0200 Subject: [PATCH 8/8] Export gitfo_shallow_exists --- src/fileops.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fileops.h b/src/fileops.h index f27f2c9e3..e20ce6f28 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -65,6 +65,7 @@ typedef struct { /* file io buffer */ } gitfo_buf; extern int gitfo_exists(const char *path); +extern int gitfo_shallow_exists(const char *path); extern int gitfo_open(const char *path, int flags); extern int gitfo_creat(const char *path, int mode); extern int gitfo_creat_force(const char *path, int mode);