Re: [PATCH] Add support for commit attributes
2014-04-10 6:25 GMT+02:00 Duy Nguyen pclo...@gmail.com: On Thu, Apr 10, 2014 at 2:38 AM, Diego Lago diego.lago.gonza...@gmail.com wrote: Commit attributes are custom commit extra headers the user can add to the commit object. The motivation for this patch is that in my company we have a custom continuous integration software that uses a custom formatted commit message (currently in YALM format) to show several information into our CI server front-end. But this YALM-based commit message pollutes the commit object not being human readable, so a good form of achieve the YALM's behaviour (without using YALM nor any other structured language) is to add custom attributes to the commit object itself. For example, in our CI server we show the risk of the change (that can be low, medium or high); we, as said before, add this information by putting YALM code inside the commit message, but the problem is that this message is not human readable. If the problem is polluting human eyes, wouldn't it be better to make git-log to filter it out? For example, we could tell git that all fields (in the message body) that start with X- are rubbish, so instead of showing X-something: base64 stuff..., it shows X-something: filtered out instead? At least people will see that this commit carries human-unreadable stuff. -- Duy Writing this data into the message, the user is forced to write it in the correct format (I think is better to write key=value pairs as an option instead of writing as message lines with spaces in key, between key and equal sign and value, and other mistakes). And is simpler to parse these attributes than the message itself. And, what if the log message is seen from the command line instead of our CI front-end? Why the CLI user (for example) should see information that does not need or does not want to see? Commit attributes are extra information, not the main information, hence this patch. PD: Sorry for the previous message not in plain text. -- Diego Lago González -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Our official home page and logo for the Git project
Andrew Ardill andrew.ard...@gmail.com writes: I think it is fair to say that the red version is the one people recognise as 'git' and so should be kept as the official version. Who is people? I never associated anything with it. I had to look at the actual web page to see what people are talking about. It's far too arbitrary and could be anything. If somebody actually took the pain and oriented the branching symbol on a suitable background shape in a manner where it formed a stylized letter G or even something obscure like a Game of Life Flier or anything, one would be closer to have something to talk about. But as it is, it is just an arbitrary dump of lines and circles with no rhyme or reason without an offset border and consequently with an edge in a saturated color bleeding unfavorably into basically every background. If that is supposed to allude to being on the bleeding edge: too smart for its own good. I mean, people discuss whether it would not be better upside down. That's nothing you would even consider if that thing had enough sensibly or recognizably arranged elements to function as an actual logo. I mean, _Emacs_ has a nice logo. And even back in the eighties, the crude kitchen sink logo it employed then was at least a good joke. I think that more effort should go into that or any other logo in order to create something identifiable and cohesive. With regard to logos, my all-time favorite still is Sun. Too bad it's history. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for commit attributes
On Thu, Apr 10, 2014 at 1:27 PM, Diego Lago González diego.lago.gonza...@gmail.com wrote: Writing this data into the message, the user is forced to write it in the correct format (I think is better to write key=value pairs as an option instead of writing as message lines with spaces in key, between key and equal sign and value, and other mistakes). And is simpler to parse these attributes than the message itself. the interpret-trailers series Christian Couder is cooking in 'pu' should handle this. And, what if the log message is seen from the command line instead of our CI front-end? Why the CLI user (for example) should see information that does not need or does not want to see? which is why git-log (and all other porcelain commands) should learn to hide the value part (but not the key part). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SUSPECT: General web-mail maintenance
General web-mail maintenance Dear Account Owner, We want to upgrade all email account scheduled for today as part of our duty to strengthen security of your mailbox. CLICK HEREhttp://www.clubseat.es/22/ to upgrade your account to Outlook Web Apps 2014. If your settings is not updated today, your account will be inactive and cannot send or receive message any longer. Sincerely, IT Helpdesk DEPT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] Verify index file before we opportunistically update it
On Thu, Apr 10, 2014 at 12:34 PM, Yiannis Marangos
yiannis.maran...@gmail.com wrote:
+/*
+ * This function verifies if index_state has the correct sha1 of an index
file.
+ * Don't die if we have any other failure, just return 0.
+ */
+static int verify_index_from(const struct index_state *istate, const char
*path)
+{
+ int fd;
+ struct stat st;
+ struct cache_header *hdr;
+ void *mmap_addr;
+ size_t mmap_size;
+
+ if (!istate-initialized)
+ return 0;
+
+ fd = open(path, O_RDONLY);
+ if (fd 0)
+ return 0;
+
+ if (fstat(fd, st))
+ return 0;
+
+ /* file is too big */
+ if (st.st_size (size_t)st.st_size)
+ return 0;
+
+ mmap_size = (size_t)st.st_size;
+ if (mmap_size sizeof(struct cache_header) + 20)
+ return 0;
+
+ mmap_addr = mmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ close(fd);
+ if (mmap_addr == MAP_FAILED)
+ return 0;
+
+ hdr = mmap_addr;
+ if (verify_hdr(hdr, mmap_size) 0)
+ goto unmap;
verify_hdr() is a bit expensive because you need to digest the whole
index file (could big as big as 14MB on webkit). Could we get away
without it? I mean, is it enough that we pick the last 20 bytes and
compare it with istate-sha1? If we only need 20 bytes, pread() may be
better than mmap().
The chance of SHA-1 collision is small enough for us to ignore, I
think. And if a client updates the index without updating the trailing
sha-1, the index is broken and we don't have to worry about
overwriting it.
+
+ if (hashcmp(istate-sha1, (unsigned char *)hdr + mmap_size - 20))
+ goto unmap;
+
+ munmap(mmap_addr, mmap_size);
+ return 1;
+
+unmap:
+ munmap(mmap_addr, mmap_size);
+ return 0;
+}
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
ZasheD
Re: [PATCH v5] Verify index file before we opportunistically update it
On Thu, Apr 10, 2014 at 05:40:55PM +0700, Duy Nguyen wrote: verify_hdr() is a bit expensive because you need to digest the whole index file (could big as big as 14MB on webkit). Could we get away without it? I mean, is it enough that we pick the last 20 bytes and compare it with istate-sha1? If we only need 20 bytes, pread() may be better than mmap(). The chance of SHA-1 collision is small enough for us to ignore, I think. And if a client updates the index without updating the trailing sha-1, the index is broken and we don't have to worry about overwriting it. That's true, I will commit version 6 in a bit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6] Verify index file before we opportunistically update it
Before we proceed to opportunistic update we must verify that the
current index file is the same as the one that we read before. There
is a possible race if we don't do this. In the example below git-status
does opportunistic update and git-rebase updates the index, but the
race can happen in general.
1. process A calls git-rebase (or does anything that uses the index)
2. process A applies 1st commit
3. process B calls git-status (or does anything that updates the index)
4. process B reads index
5. process A applies 2nd commit
6. process B takes the lock, then overwrites process A's changes.
7. process A applies 3rd commit
As an end result the 3rd commit will have a revert of the 2nd commit.
When process B takes the lock, it needs to make sure that the index
hasn't changed since step 4.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
---
Version 4 contains fixes based on Junio's comments.
Version 5 fixs a typo in commit message (git-show - git-status)
Version 6 removes verify_hdr() and use pread() instead of mmap().
cache.h | 1 +
read-cache.c | 47 ++-
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 107ac61..0460f06 100644
--- a/cache.h
+++ b/cache.h
@@ -279,6 +279,7 @@ struct index_state {
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
+ unsigned char sha1[20];
};
extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index ba13353..034865d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1477,6 +1477,7 @@ int read_index_from(struct index_state *istate, const
char *path)
if (verify_hdr(hdr, mmap_size) 0)
goto unmap;
+ hashcpy(istate-sha1, (unsigned char *)hdr + mmap_size - 20);
istate-version = ntohl(hdr-hdr_version);
istate-cache_nr = ntohl(hdr-hdr_entries);
istate-cache_alloc = alloc_nr(istate-cache_nr);
@@ -1760,6 +1761,50 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct
cache_entry *ce,
return result;
}
+/*
+ * This function verifies if index_state has the correct sha1 of an index file.
+ * Don't die if we have any other failure, just return 0.
+ */
+static int verify_index_from(const struct index_state *istate, const char
*path)
+{
+ int fd;
+ ssize_t n;
+ struct stat st;
+ unsigned char sha1[20];
+
+ if (!istate-initialized)
+ return 0;
+
+ fd = open(path, O_RDONLY);
+ if (fd 0)
+ return 0;
+
+ if (fstat(fd, st))
+ goto out;
+
+ if (st.st_size sizeof(struct cache_header) + 20)
+ goto out;
+
+ n = pread(fd, sha1, 20, st.st_size - 20);
+ if (n != 20)
+ goto out;
+
+ if (hashcmp(istate-sha1, sha1))
+ goto out;
+
+ close(fd);
+ return 1;
+
+out:
+ close(fd);
+ return 0;
+}
+
+static int verify_index(const struct index_state *istate)
+{
+ return verify_index_from(istate, get_index_file());
+}
+
static int has_racy_timestamp(struct index_state *istate)
{
int entries = istate-cache_nr;
@@ -1779,7 +1824,7 @@ static int has_racy_timestamp(struct index_state *istate)
void update_index_if_able(struct index_state *istate, struct lock_file
*lockfile)
{
if ((istate-cache_changed || has_racy_timestamp(istate))
- !write_index(istate, lockfile-fd))
+ verify_index(istate) !write_index(istate, lockfile-fd))
commit_locked_index(lockfile);
else
rollback_lock_file(lockfile);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] Verify index file before we opportunistically update it
Duy Nguyen pclo...@gmail.com writes: verify_hdr() is a bit expensive because you need to digest the whole index file (could big as big as 14MB on webkit). Could we get away without it? I mean, is it enough that we pick the last 20 bytes and compare it with istate-sha1? If we only need 20 bytes, pread() may be better than mmap(). True. I do not think we offer xread() equivalent for pread() to help coders to avoid the common mistake of failing to resume interrupted system calls, though. The only callsite of pread() we currently have is in index-pack.c, which just does n = pread(pack_fd, inbuf, n, from); if (n 0) die_errno(_(cannot pread pack file)); without paying attention to EAGAIN/EINTR, that seems wrong and may want to be fixed, before we introduce another call site of pread(). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for commit attributes
Duy Nguyen pclo...@gmail.com writes: If the problem is polluting human eyes, wouldn't it be better to make git-log to filter it out? For example, we could tell git that all fields (in the message body) that start with X- are rubbish, so instead of showing X-something: base64 stuff..., it shows X-something: filtered out instead? At least people will see that this commit carries human-unreadable stuff. We had lengthy discussions in early 2010 [*1*]. The whole thread, at least the whole sub-thread that contains the focused message, is a required reading to understand where we stand with respect to extra headers in commit objects. Any additional information about the commit can be added this patch implements is exactly the kind of thing we want to avoid, which made Linus say in an even older discussion [*2*]: No this random field could be used this random way crud, please. Even worse, the --attr pretends to be opaque by not defining what each attribute really means, but the patch hardcodes arbitrary rules like an attribute is unconditionally copied during amends and an attribute cannot be multi-valued, if I read it correctly. I actually think this recording information about commits is exactly the use-case notes were invented to address, and if it is found cumbersome to use, the reason why it is cumbersome needs to be discovered and use of notes needs to be improved. Hooks and/or a wrapper around git commit to implement their custom workflow may be involved as part of the solution and git notes may need to learn a new trick or two along the way. I am not interested in hearing let's add random crud to commit object header before let's improve notes so that it can be more smoothly used is fully explored. [References] *1* http://thread.gmane.org/gmane.comp.version-control.git/138848/focus=138892 *2* http://thread.gmane.org/gmane.comp.version-control.git/19126/focus=19149 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for commit attributes
Junio C Hamano gits...@pobox.com writes: I actually think this recording information about commits is exactly the use-case notes were invented to address, and if it is found cumbersome to use, the reason why it is cumbersome needs to be discovered and use of notes needs to be improved. Hooks and/or a wrapper around git commit to implement their custom workflow may be involved as part of the solution and git notes may need to learn a new trick or two along the way. I am not interested in hearing let's add random crud to commit object header before let's improve notes so that it can be more smoothly used is fully explored. Oh, I forgot to say that I do not have anything against embedding the extra info as part of the free-form log message text part of the commit object like you have been suggesting. If the information can be cast in stone at the commit time, that would actually be preferrable, as you do not have to worry about transferring notes separately. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/19] Portable alloca for Git
Erik Faye-Lund kusmab...@gmail.com writes: Subject: [PATCH] mingw: activate alloca Both MSVC and MINGW have alloca(3) definitions in malloc.h, so by moving win32-compat alloca.h from compat/vcbuild/include/ to compat/win32/ , which is included by both MSVC and MINGW CFLAGS, we can make alloca() work on both those Windows environments. In MINGW, malloc.h has explicit check for GNUC and if it is so, defines alloca to __builtin_alloca, so it looks like we don't need to add any code to here-shipped alloca.h to get optimum performance. Compile-tested on Windows in MSysGit. Signed-off-by: Kirill Smelkov k...@mns.spb.ru Looks good to me! Thanks; queued and pushed out on 'next'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] Add support for commit attributes
Diego Lago wrote: Commit attributes are custom commit extra headers the user can add to the commit object. The motivation for this patch is that in my company we have a custom continuous integration software that uses a custom formatted commit message (currently in YALM format) to show several information into our CI server front-end. These attributes can be used for remote-helpers as well; to store extra information that cannot be stored otherwise in Git's data structures. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase
Change delete_ref_loose()) to just flag that a ref is to be deleted but do
not actually unlink the files.
Change commit_ref_lock() so that it will unlink refs that are flagged for
deletion.
Change all callers of delete_ref_loose() to explicitely call commit_ref_lock()
to commit the deletion.
The new pattern for deleting loose refs thus become:
lock = lock_ref_sha1_basic() (or varient of)
delete_ref_loose(lock)
unlock_ref(lock) | commit_ref_lock(lock)
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 31 ---
refs.h | 2 ++
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 9771237..038614d 100644
--- a/refs.c
+++ b/refs.c
@@ -2484,16 +2484,9 @@ static int repack_without_ref(const char *refname)
static int delete_ref_loose(struct ref_lock *lock, int flag)
{
- if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
- /* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
-
- lock-lk-filename[i] = 0;
- err = unlink_or_warn(lock-lk-filename);
- lock-lk-filename[i] = '.';
- if (err errno != ENOENT)
- return 1;
- }
+ lock-delete_ref = 1;
+ lock-delete_flag = flag;
+
return 0;
}
@@ -2515,7 +2508,7 @@ int delete_ref(const char *refname, const unsigned char
*sha1, int delopt)
unlink_or_warn(git_path(logs/%s, lock-ref_name));
clear_loose_ref_cache(ref_cache);
- unlock_ref(lock);
+ ret |= commit_ref_lock(lock);
return ret;
}
@@ -2868,6 +2861,20 @@ int write_ref_sha1(struct ref_lock *lock,
int commit_ref_lock(struct ref_lock *lock)
{
+ if (lock-delete_ref) {
+ int flag = lock-delete_flag;
+
+ if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
+ /* loose */
+ int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+
+ lock-lk-filename[i] = 0;
+ err = unlink_or_warn(lock-lk-filename);
+ lock-lk-filename[i] = '.';
+ if (err errno != ENOENT)
+ return 1;
+ }
+ } else
if (!lock-skipped_write commit_ref(lock)) {
error(Couldn't set %s, lock-ref_name);
unlock_ref(lock);
@@ -3382,6 +3389,8 @@ int update_refs(const char *action, const struct
ref_update **updates_orig,
if (locks[i]) {
delnames[delnum++] = locks[i]-ref_name;
ret |= delete_ref_loose(locks[i], types[i]);
+ ret |= commit_ref_lock(locks[i]);
+ locks[i] = NULL;
}
ret |= repack_without_refs(delnames, delnum);
for (i = 0; i delnum; i++)
diff --git a/refs.h b/refs.h
index 0382128..2230d67 100644
--- a/refs.h
+++ b/refs.h
@@ -9,6 +9,8 @@ struct ref_lock {
int lock_fd;
int force_write;
int skipped_write;
+ int delete_ref;
+ int delete_flag;
};
/**
--
1.9.1.478.ga5a8238.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] refs.c: change update_refs to run the commit loops once all work is finished
During update_refs we will both be deleting refs as well as updating/changing
the refs.
Since both delete and update now use the same lock/update/commit pattern
lock = lock_ref_sha1_basic() (or varient of)
write_ref_sha1(lock)/delete_ref_loose(lock)
unlock_ref(lock) | commit_ref_lock(lock)
we can now simplify update_update refs to have one loop that locks all
involved refs.
A second loop that writes or flags for deletion, but does not commit, all
the refs.
And a final third loop that commits all the refs once all the work and
preparations are complete.
This makes updating/deleting multiple refs 'more atomic' since we will not start
the commit phase until all the preparations have completed successfully.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c
index 038614d..1678e12 100644
--- a/refs.c
+++ b/refs.c
@@ -3368,34 +3368,38 @@ int update_refs(const char *action, const struct
ref_update **updates_orig,
}
}
- /* Perform updates first so live commits remain referenced */
- for (i = 0; i n; i++)
- if (!is_null_sha1(updates[i]-new_sha1)) {
+ /* Go through and prepare all updates and deletes */
+ for (i = 0; i n; i++) {
+ if (!is_null_sha1(updates[i]-new_sha1))
ret = update_ref_write(action,
updates[i]-ref_name,
updates[i]-new_sha1,
locks[i], onerr);
- if (ret)
- unlock_ref(locks[i]);
- else
- commit_ref_lock(locks[i]);
- locks[i] = NULL;
- if (ret)
- goto cleanup;
+ else {
+ delnames[delnum++] = locks[i]-ref_name;
+ ret = delete_ref_loose(locks[i], types[i]);
}
+ if (ret)
+ goto cleanup;
+ }
- /* Perform deletes now that updates are safely completed */
+ ret = repack_without_refs(delnames, delnum);
+ for (i = 0; i delnum; i++)
+ unlink_or_warn(git_path(logs/%s, delnames[i]));
+ clear_loose_ref_cache(ref_cache);
+
+ /* Perform updates first so live commits remain referenced */
+ for (i = 0; i n; i++)
+ if (locks[i] !locks[i]-delete_ref) {
+ ret |= commit_ref_lock(locks[i]);
+ locks[i] = NULL;
+ }
+ /* And finally perform all deletes */
for (i = 0; i n; i++)
if (locks[i]) {
- delnames[delnum++] = locks[i]-ref_name;
- ret |= delete_ref_loose(locks[i], types[i]);
ret |= commit_ref_lock(locks[i]);
locks[i] = NULL;
}
- ret |= repack_without_refs(delnames, delnum);
- for (i = 0; i delnum; i++)
- unlink_or_warn(git_path(logs/%s, delnames[i]));
- clear_loose_ref_cache(ref_cache);
cleanup:
for (i = 0; i n; i++)
--
1.9.1.478.ga5a8238.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] refs.c: split writing and commiting a ref into two separate functions
Change the function write_ref_sha1() to just write the ref but not
commit the ref or the lockfile.
Add a new function commit_ref_lock() that will commit the change done by
a previous write_ref_sha1().
Update all callers of write_ref_sha1() to call commit_ref_lock().
The new pattern for updating a ref is now :
lock = lock_ref_sha1_basic() (or varient of)
write_ref_sha1(lock)
unlock_ref(lock) | commit_ref_lock(lock)
Once write_ref_sha1() returns, the new ref has been written and the lock
file has been closed.
At that stage we can then either call unlock_ref() which will abort the
update and delete the lock file withouth applying it, or call
commit_ref_lock() which will rename the lock file onto the ref file.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 10 --
builtin/commit.c | 5 +
builtin/fetch.c| 7 ++-
builtin/receive-pack.c | 4
builtin/replace.c | 6 +-
builtin/tag.c | 6 +-
fast-import.c | 7 ++-
refs.c | 39 ++-
refs.h | 4
sequencer.c| 4
walker.c | 4
11 files changed, 81 insertions(+), 15 deletions(-)
diff --git a/branch.c b/branch.c
index 660097b..903ea75 100644
--- a/branch.c
+++ b/branch.c
@@ -304,9 +304,15 @@ void create_branch(const char *head,
if (real_ref track)
setup_tracking(ref.buf + 11, real_ref, track, quiet);
- if (!dont_change_ref)
- if (write_ref_sha1(lock, sha1, msg) 0)
+ if (!dont_change_ref) {
+ if (write_ref_sha1(lock, sha1, msg) 0) {
+ unlock_ref(lock);
die_errno(_(Failed to write ref));
+ }
+ if (commit_ref_lock(lock) 0) {
+ die_errno(_(Failed to commit ref));
+ }
+ }
strbuf_release(ref);
free(real_ref);
diff --git a/builtin/commit.c b/builtin/commit.c
index d9550c5..3d8a3a8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1686,9 +1686,14 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
die(_(cannot lock HEAD ref));
}
if (write_ref_sha1(ref_lock, sha1, sb.buf) 0) {
+ unlock_ref(ref_lock);
rollback_index_files();
die(_(cannot update HEAD ref));
}
+ if (commit_ref_lock(ref_lock) 0) {
+ rollback_index_files();
+ die(_(cannot commit HEAD ref));
+ }
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..ebfb854 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -388,7 +388,12 @@ static int s_update_ref(const char *action,
if (!lock)
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
+ if (write_ref_sha1(lock, ref-new_sha1, msg) 0) {
+ unlock_ref(lock);
+ return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
+ STORE_REF_ERROR_OTHER;
+ }
+ if (commit_ref_lock(lock) 0)
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
return 0;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..4760274 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -587,8 +587,12 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
return failed to lock;
}
if (write_ref_sha1(lock, new_sha1, push)) {
+ unlock_ref(lock);
return failed to write; /* error() already called */
}
+ if (commit_ref_lock(lock))
+ return failed to commit; /* error() already called */
+
return NULL; /* good */
}
}
diff --git a/builtin/replace.c b/builtin/replace.c
index b62420a..c09ff49 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -160,8 +160,12 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
lock = lock_any_ref_for_update(ref, prev, 0, NULL);
if (!lock)
die(%s: cannot lock the ref, ref);
- if (write_ref_sha1(lock, repl, NULL) 0)
+ if (write_ref_sha1(lock, repl, NULL) 0) {
+ unlock_ref(lock);
die(%s: cannot update the ref, ref);
+ }
+ if (commit_ref_lock(lock) 0)
+ die(%s: cannot commit the ref, ref);
return 0;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 40356e3..8653a64 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -644,8 +644,12 @@ int
[PATCH v7 1/2] Add xpread() and xpwrite()
xpread() and xpwrite() pay attention to EAGAIN/EINTR, so they will resume
automatically on interrupted call.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
---
builtin/index-pack.c | 2 +-
compat/mmap.c| 4 +---
git-compat-util.h| 2 ++
wrapper.c| 36
4 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index b9f6e12..1bac0f5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -542,7 +542,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = xpread(pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
diff --git a/compat/mmap.c b/compat/mmap.c
index c9d46d1..7f662fe 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -14,7 +14,7 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
while (n length) {
- ssize_t count = pread(fd, (char *)start + n, length - n, offset
+ n);
+ ssize_t count = xpread(fd, (char *)start + n, length - n,
offset + n);
if (count == 0) {
memset((char *)start+n, 0, length-n);
@@ -22,8 +22,6 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
if (count 0) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
free(start);
errno = EACCES;
return MAP_FAILED;
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..4da04c6 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -531,6 +531,8 @@ extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
+extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
+extern ssize_t xpwrite(int fd, const void *buf, size_t len, off_t offset);
extern int xdup(int fd);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..25b7419 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -174,6 +174,42 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+/*
+ * xpread() is the same as pread(), but it automatically restarts pread()
+ * operations with a recoverable error (EAGAIN and EINTR). xpread() DOES
+ * NOT GUARANTEE that len bytes is read even if the data is available.
+ */
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
+ while (1) {
+ nr = pread(fd, buf, len, offset);
+ if ((nr 0) (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
+/*
+ * xpwrite() is the same as pwrite(), but it automatically restarts pwrite()
+ * operations with a recoverable error (EAGAIN and EINTR). xpwrite() DOES NOT
+ * GUARANTEE that len bytes is written even if the operation is successful.
+ */
+ssize_t xpwrite(int fd, const void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
+ while (1) {
+ nr = pwrite(fd, buf, len, offset);
+ if ((nr 0) (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
ssize_t read_in_full(int fd, void *buf, size_t count)
{
char *p = buf;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/2] Verify index file before we opportunistically update it
Before we proceed to opportunistic update we must verify that the
current index file is the same as the one that we read before. There
is a possible race if we don't do this. In the example below git-status
does opportunistic update and git-rebase updates the index, but the
race can happen in general.
1. process A calls git-rebase (or does anything that uses the index)
2. process A applies 1st commit
3. process B calls git-status (or does anything that updates the index)
4. process B reads index
5. process A applies 2nd commit
6. process B takes the lock, then overwrites process A's changes.
7. process A applies 3rd commit
As an end result the 3rd commit will have a revert of the 2nd commit.
When process B takes the lock, it needs to make sure that the index
hasn't changed since step 4.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
---
Version 4 contains fixes based on Junio's comments.
Version 5 fixs a typo in commit message (git-show - git-status).
Version 6 removes verify_hdr() and use pread() instead of mmap().
Version 7 use xpread() (added with [PATCH v7 1/2]) instead of pread().
cache.h | 1 +
read-cache.c | 47 ++-
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 107ac61..0460f06 100644
--- a/cache.h
+++ b/cache.h
@@ -279,6 +279,7 @@ struct index_state {
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
+ unsigned char sha1[20];
};
extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index ba13353..28de1a6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1477,6 +1477,7 @@ int read_index_from(struct index_state *istate, const
char *path)
if (verify_hdr(hdr, mmap_size) 0)
goto unmap;
+ hashcpy(istate-sha1, (unsigned char *)hdr + mmap_size - 20);
istate-version = ntohl(hdr-hdr_version);
istate-cache_nr = ntohl(hdr-hdr_entries);
istate-cache_alloc = alloc_nr(istate-cache_nr);
@@ -1760,6 +1761,50 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct
cache_entry *ce,
return result;
}
+/*
+ * This function verifies if index_state has the correct sha1 of an index file.
+ * Don't die if we have any other failure, just return 0.
+ */
+static int verify_index_from(const struct index_state *istate, const char
*path)
+{
+ int fd;
+ ssize_t n;
+ struct stat st;
+ unsigned char sha1[20];
+
+ if (!istate-initialized)
+ return 0;
+
+ fd = open(path, O_RDONLY);
+ if (fd 0)
+ return 0;
+
+ if (fstat(fd, st))
+ goto out;
+
+ if (st.st_size sizeof(struct cache_header) + 20)
+ goto out;
+
+ n = xpread(fd, sha1, 20, st.st_size - 20);
+ if (n != 20)
+ goto out;
+
+ if (hashcmp(istate-sha1, sha1))
+ goto out;
+
+ close(fd);
+ return 1;
+
+out:
+ close(fd);
+ return 0;
+}
+
+static int verify_index(const struct index_state *istate)
+{
+ return verify_index_from(istate, get_index_file());
+}
+
static int has_racy_timestamp(struct index_state *istate)
{
int entries = istate-cache_nr;
@@ -1779,7 +1824,7 @@ static int has_racy_timestamp(struct index_state *istate)
void update_index_if_able(struct index_state *istate, struct lock_file
*lockfile)
{
if ((istate-cache_changed || has_racy_timestamp(istate))
- !write_index(istate, lockfile-fd))
+ verify_index(istate) !write_index(istate, lockfile-fd))
commit_locked_index(lockfile);
else
rollback_lock_file(lockfile);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/2] Add xpread() and xpwrite()
Yiannis Marangos yiannis.maran...@gmail.com writes:
xpread() and xpwrite() pay attention to EAGAIN/EINTR, so they will resume
automatically on interrupted call.
We do not even use pwrite(); please don't add anything unnecessary
and unexercised, like xpwrite(), as potential bugs in it will go
unnoticed long after its introduction until it first gets used.
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index b9f6e12..1bac0f5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -542,7 +542,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = xpread(pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
OK.
diff --git a/compat/mmap.c b/compat/mmap.c
index c9d46d1..7f662fe 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -14,7 +14,7 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
while (n length) {
- ssize_t count = pread(fd, (char *)start + n, length - n, offset
+ n);
+ ssize_t count = xpread(fd, (char *)start + n, length - n,
offset + n);
if (count == 0) {
memset((char *)start+n, 0, length-n);
@@ -22,8 +22,6 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
if (count 0) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
free(start);
errno = EACCES;
return MAP_FAILED;
OK.
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..25b7419 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -174,6 +174,42 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+/*
+ * xpread() is the same as pread(), but it automatically restarts pread()
+ * operations with a recoverable error (EAGAIN and EINTR). xpread() DOES
+ * NOT GUARANTEE that len bytes is read even if the data is available.
+ */
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
+ while (1) {
+ nr = pread(fd, buf, len, offset);
+ if ((nr 0) (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
OK.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one
We want to make sure that we update all refs before we delete any refs
so that there is no point in time where the tips may not be reachable
from any ref in the system.
We currently acheive this by first looping over all updates and applying
them before we run a second loop and perform all the deletes.
If we sort the new sha1 for all the refs so that a deleted ref,
with sha1 0{40} comes at the end of the array, then we can just run
a single loop and still guarantee that all updates happen before
any deletes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 1678e12..453318e 100644
--- a/refs.c
+++ b/refs.c
@@ -3309,6 +3309,15 @@ static int ref_update_compare(const void *r1, const void
*r2)
return strcmp((*u1)-ref_name, (*u2)-ref_name);
}
+static int ref_delete_compare(const void *r1, const void *r2)
+{
+ const struct ref_update * const *u1 = r1;
+ const struct ref_update * const *u2 = r2;
+
+ /* -strcmp so that 0{40} sorts to the end */
+ return -strcmp((*u1)-new_sha1, (*u2)-new_sha1);
+}
+
static int ref_update_reject_duplicates(struct ref_update **updates, int n,
enum action_on_err onerr)
{
@@ -3388,13 +3397,8 @@ int update_refs(const char *action, const struct
ref_update **updates_orig,
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
- /* Perform updates first so live commits remain referenced */
- for (i = 0; i n; i++)
- if (locks[i] !locks[i]-delete_ref) {
- ret |= commit_ref_lock(locks[i]);
- locks[i] = NULL;
- }
- /* And finally perform all deletes */
+ /* Sort the array so that we perform all updates before any deletes */
+ qsort(updates, n, sizeof(*updates), ref_delete_compare);
for (i = 0; i n; i++)
if (locks[i]) {
ret |= commit_ref_lock(locks[i]);
--
1.9.1.478.ga5a8238.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Make update_refs more atomic V2
refs.c:update_refs() intermingles doing updates and checks with actually applying changes to the refs in loops that abort on error. This is done one ref at a time and means that if an error is detected that will fail the operation after only some of the ref operations have been been updated on the disk. These patches change the update and delete functions to use a three call pattern of 1, lock 2, update, or flag for deletion 3, apply on disk In the final patch I change update_refs to perform these actions in three separate loops where the final loop to 'apply on disk' all the changes will only be performed if there were no error conditions detected during any of previous loops. This should make the changes of refs in update_refs slightly more atomic. This may overlap with other current patch series for making refs updates more atomic which may mean these patches become obsolete, but I would still like some review and feedback on these changes. Version 2: Updates and fixes based on Junio's feedback. * Fix the subject line for patches so they comply with the project standard. * Redo the update/delete loops so that we maintain the correct order of operations. Perform all updates first, then perform the deletes. * Add an additional patch that allows us to do the update/delete in the correct order from within a single loop by first sorting the refs so that deletes are after all non-deletes. Ronnie Sahlberg (4): refs.c: split writing and commiting a ref into two separate functions refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase refs.c: change update_refs to run the commit loops once all work is finished refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one branch.c | 10 - builtin/commit.c | 5 +++ builtin/fetch.c| 7 +++- builtin/receive-pack.c | 4 ++ builtin/replace.c | 6 ++- builtin/tag.c | 6 ++- fast-import.c | 7 +++- refs.c | 102 + refs.h | 6 +++ sequencer.c| 4 ++ walker.c | 4 ++ 11 files changed, 123 insertions(+), 38 deletions(-) -- 1.9.1.478.ga5a8238.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/2] Add xpread() and xpwrite()
On Thu, Apr 10, 2014 at 11:35:42AM -0700, Junio C Hamano wrote: We do not even use pwrite(); please don't add anything unnecessary and unexercised, like xpwrite(), as potential bugs in it will go unnoticed long after its introduction until it first gets used. Correct, my mistake. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for commit attributes
On 10.04.2014, at 19:28, Felipe Contreras felipe.contre...@gmail.com wrote: Diego Lago wrote: Commit attributes are custom commit extra headers the user can add to the commit object. The motivation for this patch is that in my company we have a custom continuous integration software that uses a custom formatted commit message (currently in YALM format) to show several information into our CI server front-end. These attributes can be used for remote-helpers as well; to store extra information that cannot be stored otherwise in Git's data structures. +1 to that. This is reminds me of what Kiln Harmony does as part of their effort to enable full round-robin transfer between Git and Mercurial (a goal from which all other tools I know of still are far away). See http://blog.fogcreek.com/kiln-harmony-internals-the-basics/. Cheers, Max signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH 0/4] Make update_refs more atomic V2
CC'ing Michael who has been active in this area as an area expert. Ronnie, please make it a habit to run something like $ git shortlog --no-merges --since=18.months affected paths... to help you decide who your series may want to be reviewed by, before sending them. Thanks. Ronnie Sahlberg sahlb...@google.com writes: refs.c:update_refs() intermingles doing updates and checks with actually applying changes to the refs in loops that abort on error. This is done one ref at a time and means that if an error is detected that will fail the operation after only some of the ref operations have been been updated on the disk. These patches change the update and delete functions to use a three call pattern of 1, lock 2, update, or flag for deletion 3, apply on disk In the final patch I change update_refs to perform these actions in three separate loops where the final loop to 'apply on disk' all the changes will only be performed if there were no error conditions detected during any of previous loops. This should make the changes of refs in update_refs slightly more atomic. This may overlap with other current patch series for making refs updates more atomic which may mean these patches become obsolete, but I would still like some review and feedback on these changes. Version 2: Updates and fixes based on Junio's feedback. * Fix the subject line for patches so they comply with the project standard. * Redo the update/delete loops so that we maintain the correct order of operations. Perform all updates first, then perform the deletes. * Add an additional patch that allows us to do the update/delete in the correct order from within a single loop by first sorting the refs so that deletes are after all non-deletes. Ronnie Sahlberg (4): refs.c: split writing and commiting a ref into two separate functions refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase refs.c: change update_refs to run the commit loops once all work is finished refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one branch.c | 10 - builtin/commit.c | 5 +++ builtin/fetch.c| 7 +++- builtin/receive-pack.c | 4 ++ builtin/replace.c | 6 ++- builtin/tag.c | 6 ++- fast-import.c | 7 +++- refs.c | 102 + refs.h | 6 +++ sequencer.c| 4 ++ walker.c | 4 ++ 11 files changed, 123 insertions(+), 38 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 1/2] Add xpread()
xpread() pay attention to EAGAIN/EINTR, so it will resume
automatically on interrupted call.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
---
This is actually the 2nd version of this commit but before I emailed
it as version 7 because it's a dependency of [PATCH v7 2/2]. Is this
the correct way to introduce a new descendant commit? Sorry, It's my
first time that I use mailing lists for contribution.
Version 8 removes xpwrite()
builtin/index-pack.c | 2 +-
compat/mmap.c| 4 +---
git-compat-util.h| 1 +
wrapper.c| 18 ++
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index b9f6e12..1bac0f5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -542,7 +542,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = xpread(pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
diff --git a/compat/mmap.c b/compat/mmap.c
index c9d46d1..7f662fe 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -14,7 +14,7 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
while (n length) {
- ssize_t count = pread(fd, (char *)start + n, length - n, offset
+ n);
+ ssize_t count = xpread(fd, (char *)start + n, length - n,
offset + n);
if (count == 0) {
memset((char *)start+n, 0, length-n);
@@ -22,8 +22,6 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
if (count 0) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
free(start);
errno = EACCES;
return MAP_FAILED;
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..e6a4159 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -531,6 +531,7 @@ extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
+extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
extern int xdup(int fd);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..cf71817 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -174,6 +174,24 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+/*
+ * xpread() is the same as pread(), but it automatically restarts pread()
+ * operations with a recoverable error (EAGAIN and EINTR). xpread() DOES
+ * NOT GUARANTEE that len bytes is read even if the data is available.
+ */
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
+ while (1) {
+ nr = pread(fd, buf, len, offset);
+ if ((nr 0) (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
ssize_t read_in_full(int fd, void *buf, size_t count)
{
char *p = buf;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one
Ronnie Sahlberg sahlb...@google.com writes:
We want to make sure that we update all refs before we delete any refs
so that there is no point in time where the tips may not be reachable
from any ref in the system.
We currently acheive this by first looping over all updates and applying
them before we run a second loop and perform all the deletes.
If we sort the new sha1 for all the refs so that a deleted ref,
with sha1 0{40} comes at the end of the array, then we can just run
a single loop and still guarantee that all updates happen before
any deletes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 1678e12..453318e 100644
--- a/refs.c
+++ b/refs.c
@@ -3309,6 +3309,15 @@ static int ref_update_compare(const void *r1, const
void *r2)
return strcmp((*u1)-ref_name, (*u2)-ref_name);
}
+static int ref_delete_compare(const void *r1, const void *r2)
+{
+ const struct ref_update * const *u1 = r1;
+ const struct ref_update * const *u2 = r2;
+
+ /* -strcmp so that 0{40} sorts to the end */
+ return -strcmp((*u1)-new_sha1, (*u2)-new_sha1);
Two glitches:
- Didn't you mean hashcmp()?
- Don't you have an explicit -delete_ref field that is a more
direct way, than relying on the convention updating to 0{40}
is to delte, to express this?
In other words, wouldn't it be more like
return !(*u1)-delete_ref - !(*u2)-delete_ref;
or something (I may be wrong about the sign, tho---I didn't think
carefully)?
+}
+
static int ref_update_reject_duplicates(struct ref_update **updates, int n,
enum action_on_err onerr)
{
@@ -3388,13 +3397,8 @@ int update_refs(const char *action, const struct
ref_update **updates_orig,
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
- /* Perform updates first so live commits remain referenced */
- for (i = 0; i n; i++)
- if (locks[i] !locks[i]-delete_ref) {
- ret |= commit_ref_lock(locks[i]);
- locks[i] = NULL;
- }
- /* And finally perform all deletes */
+ /* Sort the array so that we perform all updates before any deletes */
+ qsort(updates, n, sizeof(*updates), ref_delete_compare);
for (i = 0; i n; i++)
if (locks[i]) {
ret |= commit_ref_lock(locks[i]);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/2] Add xpread()
Am 10.04.2014 20:54, schrieb Yiannis Marangos:
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
Odd indentation here.
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git push race condition?
On Tue, Mar 25, 2014 at 10:57 AM, Jeff King p...@peff.net wrote:
On Tue, Mar 25, 2014 at 09:45:20AM -0400, Scott Sandler wrote:
Version of git on the server? git version 1.8.3-rc0
There was significant work done between v1.8.3 and v1.8.4 on handling
races in the ref code. As I said before, I don't think the symptoms you
are describing are anything we have seen, or that could be triggered by
the races we found (which were mostly to do with ref enumeration, not
ref writing). But I would suggest upgrading to a newer version of git as
a precaution.
You mentioned elsewhere turning on the reflog, which I think is a good
idea. If there is a race of this sort, you will see a hole in the
reflog, where a ref goes from A-B, then again from A-B' (whereas with
normal writes, it would be B-B').
-Peff
This finally happened again. Here's what the reflog looks like:
2805f68 master@{0}: push
96eebc0 master@{1}: push
75bd4a6 master@{2}: push
abc30da master@{3}: push
eba874f master@{4}: push
10981e7 master@{5}: push
76b3957 master@{6}: push
2e3ea06 master@{7}: push
9d4e778 master@{8}: push
dbd70ae master@{9}: push
508ab4f master@{10}: push
36a0ce4 master@{11}: push
ddc258e master@{12}: push
cf025de master@{13}: push
dbd70ae master@{14}: push
95d33eb master@{15}: push
75b8e9a master@{16}: push
The commit that was lost does not show in the reflog at all, its short
hash was e0de949aa. The commit that won the race against it is
9d4e778 (I'm inferring this based on timing since it was pushed at
about the same time as the lost commit).
One interesting thing to note is that dbd70ae shows up at two separate
points in the reflog though, one being directly before the 9d4e778
commit that won the race. According to Gitlab's event log that commit
was just pushed once, right after 95d33eb and before cf025de as it
shows in master@{14} there. The fact that the same commit shows up
again in master@{9} is interesting.
Now that it has happened again and I've got this data, I'm going to
upgrade git but let me know if this provides any insight in the mean
time.
-Scott
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/9] Introduce publish tracking branch
As it has been discussed before, our support for triangular workflows is
lacking, and the following patch series aims to improve that situation.
We have the concept of upstream branch (e.g. 'origin/master') which is to where
our topic branches eventually should be merged to, so it makes sense that
'git rebase' uses that as the destination, but most people would not push to
such upstream branch, they would push to a publish branch
(e.g. 'github/feature-a'). We could set our upstream to the place we push, and
'git push' would be able to use that as default, and 'git branch --vv' would
show how ahead/behind we are in comparisson to that branch, but then
'git rebase' (or 'git merge') would be using the wrong branch.
This patch series adds:
1) git push --set-publish
2) git branch --set-publish
3) git branch -vv # uses and shows the publish branch when configured
4) @{publish} and @{p} marks
5) branch.$name.{push,pushremote} configurations
After this, it becomes much easier to track branches in a triangular workflow.
The publish branch is used instead of the upstream branch for tracking
information in 'git branch --vv' and 'git status' if present, otherwise there
are no changes (upstream is used).
master e230c56 [origin/master, gh/master] Git 1.8.4
* fc/publish 0a105fd [master, gh/fc/publish: ahead 1] branch: display
publish branch
fc/branch/fast 177dcad [master, gh/fc/branch/fast] branch: reorganize
verbose options
fc/trivial f289b9a [master: ahead 7] branch: trivial style fix
fc/leaksd101af4 [master: ahead 2] read-cache: plug a possible leak
stable e230c56 Git 1.8.4
Changes since v1:
* Added @{publish} and @{p} marks
Felipe Contreras (9):
push: trivial reorganization
Add concept of 'publish' branch
branch: allow configuring the publish branch
t: branch add publish branch tests
push: add --set-publish option
branch: display publish branch
sha1_name: cleanup interpret_branch_name()
sha1_name: simplify track finding
sha1_name: add support for @{publish} marks
Documentation/git-branch.txt | 11 +++
Documentation/git-push.txt | 9 +-
Documentation/revisions.txt | 4 +++
branch.c | 43 +
branch.h | 2 ++
builtin/branch.c | 74 ++
builtin/push.c | 52 +++---
remote.c | 34
remote.h | 4 +++
sha1_name.c | 62 ++--
t/t1508-at-combinations.sh | 5 +++
t/t3200-branch.sh| 76
t/t5529-push-publish.sh | 70
t/t6040-tracking-info.sh | 5 +--
transport.c | 28 ++--
transport.h | 1 +
16 files changed, 415 insertions(+), 65 deletions(-)
create mode 100755 t/t5529-push-publish.sh
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/9] push: trivial reorganization
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/push.c | 35 +++
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 0e50ddb..d10aefc 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -155,20 +155,11 @@ static NORETURN int die_push_simple(struct branch
*branch, struct remote *remote
remote-name, branch-name, advice_maybe);
}
-static const char message_detached_head_die[] =
- N_(You are not currently on a branch.\n
- To push the history leading to the current (detached HEAD)\n
- state now, use\n
- \n
- git push %s HEAD:name-of-remote-branch\n);
-
static void setup_push_upstream(struct remote *remote, struct branch *branch,
int triangular)
{
struct strbuf refspec = STRBUF_INIT;
- if (!branch)
- die(_(message_detached_head_die), remote-name);
if (!branch-merge_nr || !branch-merge || !branch-remote_name)
die(_(The current branch %s has no upstream branch.\n
To push the current branch and set the remote as upstream,
use\n
@@ -198,8 +189,6 @@ static void setup_push_upstream(struct remote *remote,
struct branch *branch,
static void setup_push_current(struct remote *remote, struct branch *branch)
{
- if (!branch)
- die(_(message_detached_head_die), remote-name);
add_refspec(branch-name);
}
@@ -240,9 +229,23 @@ static int is_workflow_triangular(struct remote *remote)
return (fetch_remote fetch_remote != remote);
}
-static void setup_default_push_refspecs(struct remote *remote)
+static const char message_detached_head_die[] =
+ N_(You are not currently on a branch.\n
+ To push the history leading to the current (detached HEAD)\n
+ state now, use\n
+ \n
+ git push %s HEAD:name-of-remote-branch\n);
+
+static struct branch *get_current_branch(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ return branch;
+}
+
+static void setup_default_push_refspecs(struct remote *remote)
+{
int triangular = is_workflow_triangular(remote);
switch (push_default) {
@@ -257,17 +260,17 @@ static void setup_default_push_refspecs(struct remote
*remote)
case PUSH_DEFAULT_SIMPLE:
if (triangular)
- setup_push_current(remote, branch);
+ setup_push_current(remote, get_current_branch(remote));
else
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_UPSTREAM:
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_CURRENT:
- setup_push_current(remote, branch);
+ setup_push_current(remote, get_current_branch(remote));
break;
case PUSH_DEFAULT_NOTHING:
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/9] branch: allow configuring the publish branch
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-branch.txt | 11 +
branch.c | 43 +
branch.h | 2 ++
builtin/branch.c | 57
4 files changed, 108 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 311b336..914fd62 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -14,7 +14,9 @@ SYNOPSIS
[(--merged | --no-merged | --contains) [commit]] [pattern...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname
[start-point]
'git branch' (--set-upstream-to=upstream | -u upstream) [branchname]
+'git branch' (--set-publish-to=publish | -p publish) [branchname]
'git branch' --unset-upstream [branchname]
+'git branch' --unset-publish [branchname]
'git branch' (-m | -M) [oldbranch] newbranch
'git branch' (-d | -D) [-r] branchname...
'git branch' --edit-description [branchname]
@@ -191,6 +193,15 @@ start-point is either a local or remote-tracking branch.
Remove the upstream information for branchname. If no branch
is specified it defaults to the current branch.
+-p publish::
+--set-publish-to=publish::
+ Set up branchname's publish tracking information. If no
+ branchname is specified, then it defaults to the current branch.
+
+--unset-publish::
+ Remove the publish information for branchname. If no branch
+ is specified it defaults to the current branch.
+
--edit-description::
Open an editor and edit the text to explain what the branch is
for, to be used by various other commands (e.g. `request-pull`).
diff --git a/branch.c b/branch.c
index 723a36b..6f4fe7f 100644
--- a/branch.c
+++ b/branch.c
@@ -144,6 +144,49 @@ static int setup_tracking(const char *new_ref, const char
*orig_ref,
return 0;
}
+void install_branch_publish(const char *name, const char *remote, const char
*remote_ref)
+{
+ struct strbuf key = STRBUF_INIT;
+
+ if (!remote !strcmp(name, remote_ref + 11) !prefixcmp(remote_ref,
refs/heads)) {
+ warning(_(Not setting branch %s as its own publish branch.),
name);
+ return;
+ }
+
+ strbuf_addf(key, branch.%s.pushremote, name);
+ git_config_set(key.buf, remote ? remote : .);
+
+ strbuf_reset(key);
+ strbuf_addf(key, branch.%s.push, name);
+ git_config_set(key.buf, remote_ref);
+
+ strbuf_release(key);
+}
+
+int setup_publish(const char *name, const char *ref)
+{
+ struct tracking tracking;
+ const char *remote, *remote_ref;
+
+ memset(tracking, 0, sizeof(tracking));
+ tracking.spec.dst = (char*)ref;
+ if (for_each_remote(find_tracked_branch, tracking))
+ return 1;
+
+ if (tracking.matches 1)
+ return error(_(Not tracking: ambiguous information for ref
%s),
+ ref);
+
+ remote = tracking.remote;
+ remote_ref = tracking.src ? tracking.src : ref;
+
+ install_branch_publish(name, remote, remote_ref);
+
+ free(tracking.src);
+
+ return 0;
+}
+
struct branch_desc_cb {
const char *config_name;
const char *value;
diff --git a/branch.h b/branch.h
index 64173ab..c9b6aa9 100644
--- a/branch.h
+++ b/branch.h
@@ -51,5 +51,7 @@ extern void install_branch_config(int flag, const char
*local, const char *origi
* Read branch description
*/
extern int read_branch_desc(struct strbuf *, const char *branch_name);
+extern int setup_publish(const char *name, const char *ref);
+extern void install_branch_publish(const char *name, const char *remote, const
char *remote_ref);
#endif
diff --git a/builtin/branch.c b/builtin/branch.c
index b4d7716..17773d7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -793,8 +793,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
int delete = 0, rename = 0, force_create = 0, list = 0;
int verbose = 0, abbrev = -1, detached = 0;
int reflog = 0, edit_description = 0;
- int quiet = 0, unset_upstream = 0;
- const char *new_upstream = NULL;
+ int quiet = 0, unset_upstream = 0, unset_publish = 0;
+ const char *new_upstream = NULL, *publish = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
struct commit_list *with_commit = NULL;
@@ -809,7 +809,9 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_SET_INT( 0, set-upstream, track, N_(change upstream
info),
BRANCH_TRACK_OVERRIDE),
OPT_STRING('u', set-upstream-to, new_upstream, upstream,
change the upstream info),
+ OPT_STRING('p', set-publish-to, publish, publish, change
the publish info),
OPT_BOOL(0, unset-upstream, unset_upstream, Unset the
upstream
[PATCH v2 2/9] Add concept of 'publish' branch
The upstream branch is:
branch.$name.remote
branch.$name.merge
The publish branch is:
branch.$name.pushremote
branch.$name.push
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/push.c | 19 +++
remote.c | 34 --
remote.h | 4
3 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index d10aefc..a1fdc49 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -192,6 +192,20 @@ static void setup_push_current(struct remote *remote,
struct branch *branch)
add_refspec(branch-name);
}
+static void setup_push_simple(struct remote *remote, struct branch *branch,
+ int triangular)
+{
+ if (branch-push_name) {
+ struct strbuf refspec = STRBUF_INIT;
+ strbuf_addf(refspec, %s:%s, branch-name, branch-push_name);
+ add_refspec(refspec.buf);
+ } else if (triangular) {
+ setup_push_current(remote, branch);
+ } else {
+ setup_push_upstream(remote, branch, triangular);
+ }
+}
+
static char warn_unspecified_push_default_msg[] =
N_(push.default is unset; its implicit value is changing in\n
Git 2.0 from 'matching' to 'simple'. To squelch this message\n
@@ -259,10 +273,7 @@ static void setup_default_push_refspecs(struct remote
*remote)
break;
case PUSH_DEFAULT_SIMPLE:
- if (triangular)
- setup_push_current(remote, get_current_branch(remote));
- else
- setup_push_upstream(remote, get_current_branch(remote),
triangular);
+ setup_push_simple(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_UPSTREAM:
diff --git a/remote.c b/remote.c
index 5f63d55..3437d1f 100644
--- a/remote.c
+++ b/remote.c
@@ -352,13 +352,17 @@ static int handle_config(const char *key, const char
*value, void *cb)
explicit_default_remote_name = 1;
}
} else if (!strcmp(subkey, .pushremote)) {
+ if (git_config_string(branch-pushremote_name, key,
value))
+ return -1;
if (branch == current_branch)
- if (git_config_string(branch_pushremote_name,
key, value))
- return -1;
+ branch_pushremote_name =
xstrdup(branch-pushremote_name);
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
add_merge(branch, xstrdup(value));
+ } else if (!strcmp(subkey, .push)) {
+ if (git_config_string(branch-push_name, key, value))
+ return -1;
}
return 0;
}
@@ -1562,6 +1566,14 @@ struct branch *branch_get(const char *name)
}
}
}
+ if (ret ret-pushremote_name) {
+ struct remote *pushremote;
+ pushremote = pushremote_get(ret-pushremote_name);
+ ret-push.src = xstrdup(ret-push_name);
+ if (remote_find_tracking(pushremote, ret-push)
+!strcmp(ret-pushremote_name, .))
+ ret-push.dst = xstrdup(ret-push_name);
+ }
return ret;
}
@@ -1771,6 +1783,15 @@ int ref_newer(const unsigned char *new_sha1, const
unsigned char *old_sha1)
return found;
}
+static char *get_base(struct branch *branch)
+{
+ if (branch-push.dst)
+ return branch-push.dst;
+ if (branch-merge branch-merge[0] branch-merge[0]-dst)
+ return branch-merge[0]-dst;
+ return NULL;
+}
+
/*
* Compare a branch with its upstream, and save their differences (number
* of commits) in *num_ours and *num_theirs.
@@ -1788,12 +1809,13 @@ int stat_tracking_info(struct branch *branch, int
*num_ours, int *num_theirs)
int rev_argc;
/* Cannot stat unless we are marked to build on top of somebody else. */
- if (!branch ||
- !branch-merge || !branch-merge[0] || !branch-merge[0]-dst)
+ if (!branch)
+ return 0;
+ base = get_base(branch);
+ if (!base)
return 0;
/* Cannot stat if what we used to build on no longer exists */
- base = branch-merge[0]-dst;
if (read_ref(base, sha1))
return -1;
theirs = lookup_commit_reference(sha1);
@@ -1869,7 +1891,7 @@ int format_tracking_info(struct branch *branch, struct
strbuf *sb)
break;
}
- base = branch-merge[0]-dst;
+ base = get_base(branch);
base = shorten_unambiguous_ref(base, 0);
if (upstream_is_gone) {
[PATCH v2 4/9] t: branch add publish branch tests
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t3200-branch.sh | 76 +++
1 file changed, 76 insertions(+)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..8cd21d1 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -907,4 +907,80 @@ test_expect_success 'tracking with unexpected .fetch
refspec' '
)
'
+test_expect_success '--set-publish-to fails on multiple branches' '
+ test_must_fail git branch --set-publish-to master a b c
+'
+
+test_expect_success '--set-publish-to fails on detached HEAD' '
+ test_when_finished git checkout master
+ git checkout master^{}
+ test_must_fail git branch --set-publish-to master
+'
+
+test_expect_success '--set-publish-to fails on a missing dst branch' '
+ test_must_fail git branch --set-publish-to master does-not-exist
+'
+
+test_expect_success '--set-publish-to fails on a missing src branch' '
+ test_must_fail git branch --set-publish-to does-not-exist master
+'
+
+test_expect_success '--set-publish-to fails on a non-ref' '
+ test_must_fail git branch --set-publish-to HEAD^{}
+'
+
+test_expect_success 'use --set-publish-to modify HEAD' '
+ git checkout master
+ test_config branch.master.pushremote foo
+ test_config branch.master.push foo
+ git branch -f test
+ git branch --set-publish-to test
+ test $(git config branch.master.pushremote) = .
+ test $(git config branch.master.push) = refs/heads/test
+'
+
+test_expect_success 'use --set-publish-to modify a particular branch' '
+ git branch -f test
+ git branch -f test2
+ git branch --set-publish-to test2 test
+ test $(git config branch.test.pushremote) = .
+ test $(git config branch.test.push) = refs/heads/test2
+'
+
+test_expect_success '--unset-publish should fail if given a non-existent
branch' '
+ test_must_fail git branch --unset-publish i-dont-exist
+'
+
+test_expect_success 'test --unset-publish on HEAD' '
+ git checkout master
+ git branch -f test
+ test_config branch.master.pushremote foo
+ test_config branch.master.push foo
+ git branch --set-publish-to test
+ git branch --unset-publish
+ test_must_fail git config branch.master.pushremote
+ test_must_fail git config branch.master.push
+ # fail for a branch without publish set
+ test_must_fail git branch --unset-publish
+'
+
+test_expect_success '--unset-publish should fail on multiple branches' '
+ test_must_fail git branch --unset-publish a b c
+'
+
+test_expect_success '--unset-publish should fail on detached HEAD' '
+ test_when_finished git checkout -
+ git checkout HEAD^{}
+ test_must_fail git branch --unset-publish
+'
+
+test_expect_success 'test --unset-publish on a particular branch' '
+ git branch -f test
+ git branch -f test2
+ git branch --set-publish-to test2 test
+ git branch --unset-publish test
+ test_must_fail git config branch.test2.pushremote
+ test_must_fail git config branch.test2.push
+'
+
test_done
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/9] branch: display publish branch
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/branch.c | 17 -
t/t6040-tracking-info.sh | 5 +++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 17773d7..e0a8d0a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -42,6 +42,7 @@ static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* LOCAL */
GIT_COLOR_GREEN,/* CURRENT */
GIT_COLOR_BLUE, /* UPSTREAM */
+ GIT_COLOR_YELLOW, /* PUBLISH */
};
enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -49,7 +50,8 @@ enum color_branch {
BRANCH_COLOR_REMOTE = 2,
BRANCH_COLOR_LOCAL = 3,
BRANCH_COLOR_CURRENT = 4,
- BRANCH_COLOR_UPSTREAM = 5
+ BRANCH_COLOR_UPSTREAM = 5,
+ BRANCH_COLOR_PUBLISH = 6
};
static enum merge_filter {
@@ -76,6 +78,8 @@ static int parse_branch_color_slot(const char *var, int ofs)
return BRANCH_COLOR_CURRENT;
if (!strcasecmp(var+ofs, upstream))
return BRANCH_COLOR_UPSTREAM;
+ if (!strcasecmp(var+ofs, publish))
+ return BRANCH_COLOR_PUBLISH;
return -1;
}
@@ -448,6 +452,17 @@ static void fill_tracking_info(struct strbuf *stat, const
char *branch_name,
else
strbuf_addstr(fancy, ref);
}
+ if (branch-push.dst) {
+ ref = shorten_unambiguous_ref(branch-push.dst, 0);
+ if (fancy.len)
+ strbuf_addstr(fancy, , );
+ if (want_color(branch_use_color))
+ strbuf_addf(fancy, %s%s%s,
+ branch_get_color(BRANCH_COLOR_PUBLISH),
+ ref,
branch_get_color(BRANCH_COLOR_RESET));
+ else
+ strbuf_addstr(fancy, ref);
+ }
if (upstream_is_gone) {
if (show_upstream_ref)
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 7ac8fd0..8b9ef63 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -33,7 +33,8 @@ test_expect_success setup '
git checkout -b b5 --track brokenbase
advance g
git branch -d brokenbase
- git checkout -b b6 origin
+ git checkout -b b6 origin
+ git branch --set-publish origin/master b6
)
git checkout -b follower --track master
advance h
@@ -64,7 +65,7 @@ b2 [origin/master: ahead 1, behind 1] d
b3 [origin/master: behind 1] b
b4 [origin/master: ahead 2] f
b5 [brokenbase: gone] g
-b6 [origin/master] c
+b6 [origin/master, origin/master] c
EOF
test_expect_success 'branch -vv' '
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/9] sha1_name: cleanup interpret_branch_name()
The 'upstream' variable doesn't hold an upstream, but a branch, so
make it clearer.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sha1_name.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 6fca869..906f09d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1057,31 +1057,31 @@ static void set_shortened_ref(struct strbuf *buf, const
char *ref)
free(s);
}
-static const char *get_upstream_branch(const char *branch_buf, int len)
+static const char *get_upstream_branch(const char *name_buf, int len)
{
- char *branch = xstrndup(branch_buf, len);
- struct branch *upstream = branch_get(*branch ? branch : NULL);
+ char *name = xstrndup(name_buf, len);
+ struct branch *branch = branch_get(*name ? name : NULL);
/*
* Upstream can be NULL only if branch refers to HEAD and HEAD
* points to something different than a branch.
*/
- if (!upstream)
+ if (!branch)
die(_(HEAD does not point to a branch));
- if (!upstream-merge || !upstream-merge[0]-dst) {
- if (!ref_exists(upstream-refname))
- die(_(No such branch: '%s'), branch);
- if (!upstream-merge) {
+ if (!branch-merge || !branch-merge[0]-dst) {
+ if (!ref_exists(branch-refname))
+ die(_(No such branch: '%s'), name);
+ if (!branch-merge) {
die(_(No upstream configured for branch '%s'),
- upstream-name);
+ branch-name);
}
die(
_(Upstream branch '%s' not stored as a remote-tracking
branch),
- upstream-merge[0]-src);
+ branch-merge[0]-src);
}
- free(branch);
+ free(name);
- return upstream-merge[0]-dst;
+ return branch-merge[0]-dst;
}
static int interpret_upstream_mark(const char *name, int namelen,
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 8/9] sha1_name: simplify track finding
It's more efficient to check for the braces first.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sha1_name.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 906f09d..aa3f3e0 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -417,7 +417,7 @@ static int ambiguous_path(const char *path, int len)
static inline int upstream_mark(const char *string, int len)
{
- const char *suffix[] = { @{upstream}, @{u} };
+ const char *suffix[] = { upstream, u };
int i;
for (i = 0; i ARRAY_SIZE(suffix); i++) {
@@ -475,7 +475,7 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
nth_prior = 1;
continue;
}
- if (!upstream_mark(str + at, len - at)) {
+ if (!upstream_mark(str + at + 2, len - at - 3))
{
reflog_len = (len-1) - (at+2);
len = at;
}
@@ -1089,7 +1089,10 @@ static int interpret_upstream_mark(const char *name, int
namelen,
{
int len;
- len = upstream_mark(name + at, namelen - at);
+ if (name[at + 1] != '{' || name[namelen - 1] != '}')
+ return -1;
+
+ len = upstream_mark(name + at + 2, namelen - at - 3);
if (!len)
return -1;
@@ -1097,7 +1100,7 @@ static int interpret_upstream_mark(const char *name, int
namelen,
return -1;
set_shortened_ref(buf, get_upstream_branch(name, at));
- return len + at;
+ return len + at + 3;
}
/*
--
1.9.1+fc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/9] push: add --set-publish option
To setup publish tracking branch, like 'git branch --set-publish'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-push.txt | 9 +-
builtin/push.c | 2 ++
t/t5529-push-publish.sh| 70 ++
transport.c| 28 +--
transport.h| 1 +
5 files changed, 100 insertions(+), 10 deletions(-)
create mode 100755 t/t5529-push-publish.sh
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2b7f4f9..bf6ff9c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
- [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u
| --set-upstream]
+ [--repo=repository] [-f | --force] [--prune] [-v | --verbose]
+ [-u | --set-upstream] [-p | --set-publish]
[--force-with-lease[=refname[:expect]]]
[--no-verify] [repository [refspec...]]
@@ -231,6 +232,12 @@ useful if you write an alias or script around 'git push'.
linkgit:git-pull[1] and other commands. For more information,
see 'branch.name.merge' in linkgit:git-config[1].
+-p::
+--set-publish::
+ For every branch that is up to date or successfully pushed, add
+ publish branch tracking reference, used by argument-less
+ linkgit:git-pull[1] and other commands.
+
--[no-]thin::
These options are passed to linkgit:git-send-pack[1]. A thin transfer
significantly reduces the amount of sent data when the sender and
diff --git a/builtin/push.c b/builtin/push.c
index a1fdc49..9e61b8f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -532,6 +532,8 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
OPT_STRING( 0 , exec, receivepack, receive-pack,
N_(receive pack program)),
OPT_BIT('u', set-upstream, flags, N_(set upstream for git
pull/status),
TRANSPORT_PUSH_SET_UPSTREAM),
+ OPT_BIT('p', set-publish, flags, N_(set publish for git
pull/status),
+ TRANSPORT_PUSH_SET_PUBLISH),
OPT_BOOL(0, progress, progress, N_(force progress
reporting)),
OPT_BIT(0, prune, flags, N_(prune locally removed refs),
TRANSPORT_PUSH_PRUNE),
diff --git a/t/t5529-push-publish.sh b/t/t5529-push-publish.sh
new file mode 100755
index 000..2037026
--- /dev/null
+++ b/t/t5529-push-publish.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='push with --set-publish'
+
+. ./test-lib.sh
+
+test_expect_success 'setup bare parent' '
+ git init --bare parent
+ git remote add publish parent
+'
+
+test_expect_success 'setup local commit' '
+ echo content file
+ git add file
+ git commit -m one
+'
+
+check_config() {
+ (echo $2; echo $3) expect.$1
+ (git config branch.$1.pushremote
+git config branch.$1.push) actual.$1
+ test_cmp expect.$1 actual.$1
+}
+
+test_expect_success 'push -p master:master' '
+ git push -p publish master:master
+ check_config master publish refs/heads/master
+'
+
+test_expect_success 'push -u master:other' '
+ git push -p publish master:other
+ check_config master publish refs/heads/other
+'
+
+test_expect_success 'push -p --dry-run master:otherX' '
+ git push -p --dry-run publish master:otherX
+ check_config master publish refs/heads/other
+'
+
+test_expect_success 'push -p master2:master2' '
+ git branch master2
+ git push -p publish master2:master2
+ check_config master2 publish refs/heads/master2
+'
+
+test_expect_success 'push -p master2:other2' '
+ git push -p publish master2:other2
+ check_config master2 publish refs/heads/other2
+'
+
+test_expect_success 'push -p :master2' '
+ git push -p publish :master2
+ check_config master2 publish refs/heads/other2
+'
+
+test_expect_success 'push -u --all' '
+ git branch all1
+ git branch all2
+ git push -p --all
+ check_config all1 publish refs/heads/all1
+ check_config all2 publish refs/heads/all2
+'
+
+test_expect_success 'push -p HEAD' '
+ git checkout -b headbranch
+ git push -p publish HEAD
+ check_config headbranch publish refs/heads/headbranch
+'
+
+test_done
diff --git a/transport.c b/transport.c
index ca7bb44..dfcddfe 100644
--- a/transport.c
+++ b/transport.c
@@ -143,8 +143,8 @@ static void insert_packed_refs(const char *packed_refs,
struct ref **list)
}
}
-static void set_upstreams(struct transport *transport, struct ref *refs,
- int pretend)
+static void set_tracking(struct transport *transport, struct ref *refs,
+ int pretend, int publish)
{
struct ref *ref;
for (ref = refs; ref; ref = ref-next) {
@@
[PATCH v2 9/9] sha1_name: add support for @{publish} marks
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/revisions.txt | 4
sha1_name.c | 49 -
t/t1508-at-combinations.sh | 5 +
3 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 5a286d0..fd01cb4 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -96,6 +96,10 @@ some output processing may assume ref names in UTF-8.
refers to the branch that the branch specified by branchname is set to build
on
top of. A missing branchname defaults to the current one.
+'branchname@\{publish\}', e.g. 'master@\{publish\}', '@\{p\}'::
+ The suffix '@\{publish\}' to a branchname refers to the remote branch to
+ push to. A missing branchname defaults to the current one.
+
'rev{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
A suffix '{caret}' to a revision parameter means the first parent of
that commit object. '{caret}n' means the nth parent (i.e.
diff --git a/sha1_name.c b/sha1_name.c
index aa3f3e0..a36852d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -415,9 +415,9 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-static inline int upstream_mark(const char *string, int len)
+static inline int tracking_mark(const char *string, int len)
{
- const char *suffix[] = { upstream, u };
+ const char *suffix[] = { upstream, u, publish, p };
int i;
for (i = 0; i ARRAY_SIZE(suffix); i++) {
@@ -475,7 +475,7 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
nth_prior = 1;
continue;
}
- if (!upstream_mark(str + at + 2, len - at - 3))
{
+ if (!tracking_mark(str + at + 2, len - at - 3))
{
reflog_len = (len-1) - (at+2);
len = at;
}
@@ -1057,10 +1057,11 @@ static void set_shortened_ref(struct strbuf *buf, const
char *ref)
free(s);
}
-static const char *get_upstream_branch(const char *name_buf, int len)
+static const char *get_tracking_branch(const char *name_buf, int len, char
type)
{
char *name = xstrndup(name_buf, len);
struct branch *branch = branch_get(*name ? name : NULL);
+ char *tracking = NULL;
/*
* Upstream can be NULL only if branch refers to HEAD and HEAD
@@ -1068,23 +1069,35 @@ static const char *get_upstream_branch(const char
*name_buf, int len)
*/
if (!branch)
die(_(HEAD does not point to a branch));
- if (!branch-merge || !branch-merge[0]-dst) {
- if (!ref_exists(branch-refname))
- die(_(No such branch: '%s'), name);
- if (!branch-merge) {
- die(_(No upstream configured for branch '%s'),
- branch-name);
+ switch (type) {
+ case 'u':
+ if (!branch-merge || !branch-merge[0]-dst) {
+ if (!ref_exists(branch-refname))
+ die(_(No such branch: '%s'), name);
+ if (!branch-merge) {
+ die(_(No upstream configured for branch '%s'),
+ branch-name);
+ }
+ die(
+ _(Upstream branch '%s' not stored as a
remote-tracking branch),
+ branch-merge[0]-src);
+ }
+ tracking = branch-merge[0]-dst;
+ break;
+ case 'p':
+ if (!branch-push.dst) {
+ die(_(No publish configured for branch '%s'),
+ branch-name);
}
- die(
- _(Upstream branch '%s' not stored as a remote-tracking
branch),
- branch-merge[0]-src);
+ tracking = branch-push.dst;
+ break;
}
free(name);
- return branch-merge[0]-dst;
+ return tracking;
}
-static int interpret_upstream_mark(const char *name, int namelen,
+static int interpret_tracking_mark(const char *name, int namelen,
int at, struct strbuf *buf)
{
int len;
@@ -1092,14 +1105,14 @@ static int interpret_upstream_mark(const char *name,
int namelen,
if (name[at + 1] != '{' || name[namelen - 1] != '}')
return -1;
- len = upstream_mark(name + at + 2, namelen - at - 3);
+ len = tracking_mark(name + at + 2, namelen - at - 3);
if (!len)
return -1;
if (memchr(name, ':', at))
return -1;
-
Re: [PATCH v8 1/2] Add xpread()
Johannes Sixt j...@kdbg.org writes:
Am 10.04.2014 20:54, schrieb Yiannis Marangos:
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ssize_t nr;
+if (len MAX_IO_SIZE)
+len = MAX_IO_SIZE;
Odd indentation here.
-- Hannes
Good eyes, even though this is copypasting an existing error from
surrounding code ;-)
I'll queue this instead (rebased on top of maint-1.8.5 even though I
doubt we would be issuing 1.8.5.6).
-- 8 --
From: Yiannis Marangos yiannis.maran...@gmail.com
Date: Thu, 10 Apr 2014 21:54:12 +0300
Subject: [PATCH] wrapper.c: add xpread() similar to xread()
It is a common mistake to call read(2)/pread(2) and forget to
anticipate that they may return error with EAGAIN/EINTR when the
system call is interrupted.
We have xread() helper to relieve callers of read(2) from having to
worry about it; add xpread() helper to do the same for pread(2).
Update the caller in the builtin/index-pack.c and the mmap emulation
in compat/.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/index-pack.c | 2 +-
compat/mmap.c| 4 +---
git-compat-util.h| 1 +
wrapper.c| 18 ++
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 9e9eb4b..e7a6b53 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -542,7 +542,7 @@ static void *unpack_data(struct object_entry *obj,
do {
ssize_t n = (len 64*1024) ? len : 64*1024;
- n = pread(pack_fd, inbuf, n, from);
+ n = xpread(pack_fd, inbuf, n, from);
if (n 0)
die_errno(_(cannot pread pack file));
if (!n)
diff --git a/compat/mmap.c b/compat/mmap.c
index c9d46d1..7f662fe 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -14,7 +14,7 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
while (n length) {
- ssize_t count = pread(fd, (char *)start + n, length - n, offset
+ n);
+ ssize_t count = xpread(fd, (char *)start + n, length - n,
offset + n);
if (count == 0) {
memset((char *)start+n, 0, length-n);
@@ -22,8 +22,6 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
}
if (count 0) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
free(start);
errno = EACCES;
return MAP_FAILED;
diff --git a/git-compat-util.h b/git-compat-util.h
index 7776f12..9eec5fb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -534,6 +534,7 @@ extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
+extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
extern int xdup(int fd);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..5b3c7fc 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -174,6 +174,24 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+/*
+ * xpread() is the same as pread(), but it automatically restarts pread()
+ * operations with a recoverable error (EAGAIN and EINTR). xpread() DOES
+ * NOT GUARANTEE that len bytes is read even if the data is available.
+ */
+ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
+{
+ ssize_t nr;
+ if (len MAX_IO_SIZE)
+ len = MAX_IO_SIZE;
+ while (1) {
+ nr = pread(fd, buf, len, offset);
+ if ((nr 0) (errno == EAGAIN || errno == EINTR))
+ continue;
+ return nr;
+ }
+}
+
ssize_t read_in_full(int fd, void *buf, size_t count)
{
char *p = buf;
--
1.9.2-590-g468068b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Ramkumar Ramachandra wrote:
We already have a branch.*.pushremote, and I don't see the value of
branch.*.pushbranch (what you're referring to as pushupstream, I assume)
except for Gerrit users. Frankly, I don't use full triangular workflows
myself mainly because my prompt is compromised: when I have a branch.*.remote
different from branch.*.pushremote, I'd like to see where my branch is with
respect to @{u} and @{publish} (not yet invented);
@{publish} not yet invented? I sent this back in October:
http://article.gmane.org/gmane.comp.version-control.git/235981
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/2] Verify index file before we opportunistically update it
Yiannis Marangos yiannis.maran...@gmail.com writes:
+ n = xpread(fd, sha1, 20, st.st_size - 20);
+ if (n != 20)
+ goto out;
I think it is possible for pread(2) to give you a short-read.
The existing callers of emulated mmap and index-pack are prepared to
handle a short-read correctly, but I do not think this code does.
I'll queue this instead in the meantime.
-- 8 --
From: Yiannis Marangos yiannis.maran...@gmail.com
Date: Thu, 10 Apr 2014 21:31:21 +0300
Subject: [PATCH] read-cache.c: verify index file before we opportunistically
update it
Before we proceed to opportunistically update the index (often done
by an otherwise read-only operation like git status and git diff
that internally refreshes the index), we must verify that the
current index file is the same as the one that we read earlier
before we took the lock on it, in order to avoid a possible race.
In the example below git-status does opportunistic update and
git-rebase updates the index, but the race can happen in general.
1. process A calls git-rebase (or does anything that uses the index)
2. process A applies 1st commit
3. process B calls git-status (or does anything that updates the index)
4. process B reads index
5. process A applies 2nd commit
6. process B takes the lock, then overwrites process A's changes.
7. process A applies 3rd commit
As an end result the 3rd commit will have a revert of the 2nd commit.
When process B takes the lock, it needs to make sure that the index
hasn't changed since step 4.
Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 3 +++
read-cache.c | 47 ++-
wrapper.c| 20
3 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index ce377e1..9244c38 100644
--- a/cache.h
+++ b/cache.h
@@ -279,6 +279,7 @@ struct index_state {
initialized : 1;
struct hash_table name_hash;
struct hash_table dir_hash;
+ unsigned char sha1[20];
};
extern struct index_state the_index;
@@ -1199,6 +1200,8 @@ extern void fsync_or_die(int fd, const char *);
extern ssize_t read_in_full(int fd, void *buf, size_t count);
extern ssize_t write_in_full(int fd, const void *buf, size_t count);
+extern ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
+
static inline ssize_t write_str_in_full(int fd, const char *str)
{
return write_in_full(fd, str, strlen(str));
diff --git a/read-cache.c b/read-cache.c
index 33dd676..f4a0d61 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1464,6 +1464,7 @@ int read_index_from(struct index_state *istate, const
char *path)
if (verify_hdr(hdr, mmap_size) 0)
goto unmap;
+ hashcpy(istate-sha1, (unsigned char *)hdr + mmap_size - 20);
istate-version = ntohl(hdr-hdr_version);
istate-cache_nr = ntohl(hdr-hdr_entries);
istate-cache_alloc = alloc_nr(istate-cache_nr);
@@ -1747,6 +1748,50 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct
cache_entry *ce,
return result;
}
+/*
+ * This function verifies if index_state has the correct sha1 of the
+ * index file. Don't die if we have any other failure, just return 0.
+ */
+static int verify_index_from(const struct index_state *istate, const char
*path)
+{
+ int fd;
+ ssize_t n;
+ struct stat st;
+ unsigned char sha1[20];
+
+ if (!istate-initialized)
+ return 0;
+
+ fd = open(path, O_RDONLY);
+ if (fd 0)
+ return 0;
+
+ if (fstat(fd, st))
+ goto out;
+
+ if (st.st_size sizeof(struct cache_header) + 20)
+ goto out;
+
+ n = pread_in_full(fd, sha1, 20, st.st_size - 20);
+ if (n != 20)
+ goto out;
+
+ if (hashcmp(istate-sha1, sha1))
+ goto out;
+
+ close(fd);
+ return 1;
+
+out:
+ close(fd);
+ return 0;
+}
+
+static int verify_index(const struct index_state *istate)
+{
+ return verify_index_from(istate, get_index_file());
+}
+
static int has_racy_timestamp(struct index_state *istate)
{
int entries = istate-cache_nr;
@@ -1766,7 +1811,7 @@ static int has_racy_timestamp(struct index_state *istate)
void update_index_if_able(struct index_state *istate, struct lock_file
*lockfile)
{
if ((istate-cache_changed || has_racy_timestamp(istate))
- !write_index(istate, lockfile-fd))
+ verify_index(istate) !write_index(istate, lockfile-fd))
commit_locked_index(lockfile);
else
rollback_lock_file(lockfile);
diff --git a/wrapper.c b/wrapper.c
index 5b3c7fc..bc1bfb8 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -232,6 +232,26 @@ ssize_t write_in_full(int fd, const void *buf, size_t
count)
return total;
}
+ssize_t pread_in_full(int fd, void *buf, size_t
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Ramkumar Ramachandra wrote:
My primary concern is that the proposed @{publish} should be a first-class
citizen; if it has everything that @{u} has, then we're both good: you'd
primarily use @{u}, while I'd primarily use @{publish}.
Something like this?
http://article.gmane.org/gmane.comp.version-control.git/246038
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one
On Thu, Apr 10, 2014 at 12:01 PM, Junio C Hamano gits...@pobox.com wrote:
Ronnie Sahlberg sahlb...@google.com writes:
We want to make sure that we update all refs before we delete any refs
so that there is no point in time where the tips may not be reachable
from any ref in the system.
We currently acheive this by first looping over all updates and applying
them before we run a second loop and perform all the deletes.
If we sort the new sha1 for all the refs so that a deleted ref,
with sha1 0{40} comes at the end of the array, then we can just run
a single loop and still guarantee that all updates happen before
any deletes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 1678e12..453318e 100644
--- a/refs.c
+++ b/refs.c
@@ -3309,6 +3309,15 @@ static int ref_update_compare(const void *r1, const
void *r2)
return strcmp((*u1)-ref_name, (*u2)-ref_name);
}
+static int ref_delete_compare(const void *r1, const void *r2)
+{
+ const struct ref_update * const *u1 = r1;
+ const struct ref_update * const *u2 = r2;
+
+ /* -strcmp so that 0{40} sorts to the end */
+ return -strcmp((*u1)-new_sha1, (*u2)-new_sha1);
Two glitches:
- Didn't you mean hashcmp()?
- Don't you have an explicit -delete_ref field that is a more
direct way, than relying on the convention updating to 0{40}
is to delte, to express this?
In other words, wouldn't it be more like
return !(*u1)-delete_ref - !(*u2)-delete_ref;
or something (I may be wrong about the sign, tho---I didn't think
carefully)?
I don't have access to delete_ref from 'struct ref_update'.
But much worse is that the patch is completely bogus.
Sorting and changing the order of the items in the updates[] array
does not affect the ordering of the items in the locks[] array
so this can not work.
I will delete this bogus patch from any new version of the patch series.
regards
ronnie sahlberg
+}
+
static int ref_update_reject_duplicates(struct ref_update **updates, int n,
enum action_on_err onerr)
{
@@ -3388,13 +3397,8 @@ int update_refs(const char *action, const struct
ref_update **updates_orig,
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
- /* Perform updates first so live commits remain referenced */
- for (i = 0; i n; i++)
- if (locks[i] !locks[i]-delete_ref) {
- ret |= commit_ref_lock(locks[i]);
- locks[i] = NULL;
- }
- /* And finally perform all deletes */
+ /* Sort the array so that we perform all updates before any deletes */
+ qsort(updates, n, sizeof(*updates), ref_delete_compare);
for (i = 0; i n; i++)
if (locks[i]) {
ret |= commit_ref_lock(locks[i]);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Make update_refs more atomic V2
Junio C Hamano gits...@pobox.com writes: CC'ing Michael who has been active in this area as an area expert. Ronnie, please make it a habit to run something like $ git shortlog --no-merges --since=18.months affected paths... to help you decide who your series may want to be reviewed by, before sending them. Thanks. Another tip. git format-patch -v3 would produce Subject: [PATCH v3 0/4] blah blah blah... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] Introduce publish tracking branch
A handful of minimum tweaks [*1*] here and there were necessary in order to queue the series on 'pu', but to me, the feature looked like quite a straight-forward addition. I'd be dropping the jk/branch-at-publish-rebased from 'pu', at least tentatively, as that one was primarily Peff giving Ram a base to build on top to achieve essentially the same thing as this series does. I didn't bother to check if this series could have reused some from that series (primarily because I was short of time, had to take the work laptop to service, etc. etc.) before doing so. I didn't think too deeply about the workflow ramifications of this series brings in, either---that is left for the reviewers Cc'ed on the patches. [Footnote] *1* Things like these: - the context in builtin/push.c has already changed at the tip of 'master' (we already pretend to be Git 2.0) and the patch text needed to be adjusted. - an instance of cast to (char*) fixed to (char *). - t/t5529 is already used by other topics, renaming the new test to t/t5534-push-publish.sh. - !prefixcmp() is already removed for Git 2.0, replacing its use with starts_with(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks
Felipe Contreras wrote:
@@ -1068,23 +1069,35 @@ static const char *get_upstream_branch(const char
*name_buf, int len)
*/
if (!branch)
die(_(HEAD does not point to a branch));
- if (!branch-merge || !branch-merge[0]-dst) {
- if (!ref_exists(branch-refname))
- die(_(No such branch: '%s'), name);
- if (!branch-merge) {
- die(_(No upstream configured for branch '%s'),
- branch-name);
+ switch (type) {
+ case 'u':
+ if (!branch-merge || !branch-merge[0]-dst) {
+ if (!ref_exists(branch-refname))
+ die(_(No such branch: '%s'), name);
+ if (!branch-merge) {
+ die(_(No upstream configured for branch
'%s'),
+ branch-name);
+ }
+ die(
+ _(Upstream branch '%s' not stored as a
remote-tracking branch),
+ branch-merge[0]-src);
+ }
+ tracking = branch-merge[0]-dst;
+ break;
+ case 'p':
+ if (!branch-push.dst) {
+ die(_(No publish configured for branch '%s'),
+ branch-name);
This assumes a push.default value of 'current' or 'matching'. What
happens if push.default is set to 'nothing' or 'upstream', for
instance?
p.s- Good to see you back on the list :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 8/9] sha1_name: simplify track finding
Felipe Contreras wrote: It's more efficient to check for the braces first. Why is it more efficient? So you can error out quickly in the case of a malformed string? I'm personally not thrilled about this change. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/9] sha1_name: cleanup interpret_branch_name()
Felipe Contreras wrote: sha1_name.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) I like this variable rename. This instance has annoyed me in the past. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks
Felipe Contreras wrote:
diff --git a/sha1_name.c b/sha1_name.c
index aa3f3e0..a36852d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -415,9 +415,9 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-static inline int upstream_mark(const char *string, int len)
+static inline int tracking_mark(const char *string, int len)
{
- const char *suffix[] = { upstream, u };
+ const char *suffix[] = { upstream, u, publish, p };
Oh, another thing: on some threads, people decided that @{push}
would be a more apt name (+ alias @{u} to @{pull} for symmetry).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/9] branch: display publish branch
Felipe Contreras wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Please write a commit message, preferably showing the new git-branch output. I noticed that this only picks up a publish-branch if branch.*.pushremote is configured. What happened to the case when remote.pushdefault is configured? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
@@ -1068,23 +1069,35 @@ static const char *get_upstream_branch(const char
*name_buf, int len)
*/
if (!branch)
die(_(HEAD does not point to a branch));
- if (!branch-merge || !branch-merge[0]-dst) {
- if (!ref_exists(branch-refname))
- die(_(No such branch: '%s'), name);
- if (!branch-merge) {
- die(_(No upstream configured for branch '%s'),
- branch-name);
+ switch (type) {
+ case 'u':
+ if (!branch-merge || !branch-merge[0]-dst) {
+ if (!ref_exists(branch-refname))
+ die(_(No such branch: '%s'), name);
+ if (!branch-merge) {
+ die(_(No upstream configured for branch
'%s'),
+ branch-name);
+ }
+ die(
+ _(Upstream branch '%s' not stored as a
remote-tracking branch),
+ branch-merge[0]-src);
+ }
+ tracking = branch-merge[0]-dst;
+ break;
+ case 'p':
+ if (!branch-push.dst) {
+ die(_(No publish configured for branch '%s'),
+ branch-name);
This assumes a push.default value of 'current' or 'matching'. What
happens if push.default is set to 'nothing' or 'upstream', for
instance?
Why would that matter? @{upstream} doesn't depend on this, neither does
@{publish}; @{upstream} is .remote+.merge, @{publish} is .pushremote+.push.
If the user hasn't configured a publish branch, @{publish} fails.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 8/9] sha1_name: simplify track finding
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
It's more efficient to check for the braces first.
Why is it more efficient? So you can error out quickly in the case of
a malformed string?
That's one reason. The other is that get_sha1_basic() calls upstream_mark()
when we _already_ know there's a @{foo}.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
diff --git a/sha1_name.c b/sha1_name.c
index aa3f3e0..a36852d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -415,9 +415,9 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-static inline int upstream_mark(const char *string, int len)
+static inline int tracking_mark(const char *string, int len)
{
- const char *suffix[] = { upstream, u };
+ const char *suffix[] = { upstream, u, publish, p };
Oh, another thing: on some threads, people decided that @{push}
would be a more apt name (+ alias @{u} to @{pull} for symmetry).
@{push} is the name I originally suggested, but it's weird to talk about
the push branch, so I decided on the publish branch, which is more natural.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/9] branch: display publish branch
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Please write a commit message, preferably showing the new git-branch output.
Yeah... this has been sitting in git-fc for quite a while, I wasn't expecting
to send this patch series again given that nobody commented on v1.
I noticed that this only picks up a publish-branch if
branch.*.pushremote is configured. What happened to the case when
remote.pushdefault is configured?
What happens when branch.*.remote is not configured for @{upstream}? The same
thing.
It might be useful to visualize what would be the name of the branch when
pushing it (without a refspec) even if the publish branch hasn't been
configured, but I think the code would be much more coplicated, and it would
break symetry with @{upstream}, besides, the user can just do 'git push -p
branch', and from that moment on it will be visible.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
Get a loan @ 2% interest rate with Mercantile Finance,Blacklisted are welcome.For more info:email: mercantilefinanceloanserv...@webmail.co.za -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/2] Verify index file before we opportunistically update it
On Fri, Apr 11, 2014 at 2:28 AM, Junio C Hamano gits...@pobox.com wrote: Yiannis Marangos yiannis.maran...@gmail.com writes: + n = xpread(fd, sha1, 20, st.st_size - 20); + if (n != 20) + goto out; I think it is possible for pread(2) to give you a short-read. The existing callers of emulated mmap and index-pack are prepared to handle a short-read correctly, but I do not think this code does. I'll queue this instead in the meantime. There are two things to sort out (sorry I can't spend much time on it right now): should the same sha1 test be done in write_index(), in addition to update_index_if_able(). And what do we do about istate-sha1[] in discard_index(), keep it or clear it. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
