Re: [PATCH] lookup_object: prioritize recently found objects
On Fri, May 03, 2013 at 07:59:09AM +0200, Johannes Sixt wrote: Am 5/2/2013 17:46, schrieb Jeff King: On Thu, May 02, 2013 at 09:05:01AM +0200, Johannes Sixt wrote: BTW, do you notice that the function is now modifying an object (the hash table) even though this is rather unexpected from a lookup function? I think this is fine. The function is conceptually constant from the outside; callers don't even know about the hash table. They just know that there is some mapping. It's similar to the way that lookup_commit will lazily allocate the struct commit. The callers do not care whether it exists already or not; they care that at the end of the function, they have a pointer to the commit. Everything else is an implementation detail. Can we be sure that the function is never invoked in concurrently from different threads? I attempted to audit code paths, but quickly gave up because I know too little about this machinery. I didn't check explicitly, but in general such a program would already need a mutex to synchronize object lookup. Not for lookup_object specifically, but because lookup_object is mostly used to back lookup_commit, lookup_tree, etc, which are already not re-entrant (because they lazily insert into the hash behind the scenes). -Peff -- 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] lookup_object: prioritize recently found objects
On Fri, May 03, 2013 at 02:02:44AM -0400, Jeff King wrote: Can we be sure that the function is never invoked in concurrently from different threads? I attempted to audit code paths, but quickly gave up because I know too little about this machinery. I didn't check explicitly, but in general such a program would already need a mutex to synchronize object lookup. Not for lookup_object specifically, but because lookup_object is mostly used to back lookup_commit, lookup_tree, etc, which are already not re-entrant (because they lazily insert into the hash behind the scenes). I just looked through the 25 existing calls to lookup_object. All of them should be OK. Most of them are coupled with immediate calls to update the hash table and/or to call read_sha1_file (which is also very not-thread-safe). So I don't think the patch introduces any bug into the current code. It does introduce a potential for future bugs in concurrent code, but I don't know that it makes a significant dent in the current codebase. We already have a lot of non-reentrant functions, including all of the other lookup_* functions. Our concurrent code is typically very careful to use a small known-safe subset of functions. I did look originally at updating the ordering when the hash is already being updated (i.e., to insert a new entry at the front of a collision chain, rather than the back). However, that didn't yield nearly as much improvement. I think partially because the locality of an insert/lookup pair is not as good as a lookup/lookup pair. And partially because we destroy that ordering for existing entries whenever we grow the hash table. -Peff -- 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: Another use of @?
Duy Nguyen pclo...@gmail.com writes: My setup is a bit peculiar where I do git development on three different machines. Say I updated branch long-branch-name on machine A. Then I continue my work on machine B. I would want to hard reset that long-branch-name on machine B before resuming my work. What I usually do is git co long-branch-name git diff A/long-branch-name git reset --hard A/long-branch-name Perhaps git checkout long-braTAB git diff A/!$ git reset --hard !$ In any case, not a Git question, I would have to say. -- 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 4/5] index-pack, unpack-objects: add --not-so-strict for connectivity check
Duy Nguyen pclo...@gmail.com writes: What do you mean by partial history? Do we have dangling pointers after doing that commit walker? ^C will leave the objects and it is safe because it will not update refs. But your code that does not verify the full connectivity from such an object (that is outside the transferred pack) to the rest of the history will then make the resulting repository broken, if you update your ref to point at such an object, no? Ading a single has_sha1_file() only verifies that single object, not the history behind it. -- 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
links of london Meant for Way Partners
A good cutting edge on line store by using a main difference, you bet What i'm saying is any * links of london sale http://www.linksoflondonoutletstore.co.uk/ * . It's an individual retail outlet the fact that justifies way that will a a fact feel. Pc training courses merchandise or simply earrings the quality of any importance for layout together with superior the fact that cannot be validated by just key phrases. Any retail outlet was initially open during the 1990, primarily giving to those who for The country even so it immediately attained schedule. * http://www.linksoflondonoutletstore.co.uk/ * -- View this message in context: http://git.661346.n2.nabble.com/links-of-london-Meant-for-Way-Partners-tp7584821.html Sent from the git mailing list archive at Nabble.com. -- 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: Another use of @?
Junio C Hamano gits...@pobox.com writes:
Duy Nguyen pclo...@gmail.com writes:
My setup is a bit peculiar where I do git development on three
different machines. Say I updated branch long-branch-name on machine
A. Then I continue my work on machine B. I would want to hard reset
that long-branch-name on machine B before resuming my work. What I
usually do is
git co long-branch-name
git diff A/long-branch-name
git reset --hard A/long-branch-name
Perhaps
git checkout long-braTAB
git diff A/!$
git reset --hard !$
In any case, not a Git question, I would have to say.
As a Git question, probably the answers are
git co long-braTAB
git diff @{u}
git reset --hard @{u}
with an appropriate setting of the upstream, perhaps?
--
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: Another use of @?
On Thu, May 2, 2013 at 9:51 PM, Duy Nguyen pclo...@gmail.com wrote:
Hi,
My setup is a bit peculiar where I do git development on three
different machines. Say I updated branch long-branch-name on machine
A. Then I continue my work on machine B. I would want to hard reset
that long-branch-name on machine B before resuming my work. What I
usually do is
git co long-branch-name
git diff A/long-branch-name
git reset --hard A/long-branch-name
but typing long-branch-name (even with TAB completion) is not fun.
Could I do this (or something similar) instead?
git co long-branch-name
git diff A/@
git reset --hard A/@
Maybe this would make more sense:
%git co long-branch-name
%git reset --keep A/long-branch-name
If you have changes but they don't conflict, they will be carried
over, and it they do conflict, the reset won't continue. I think in
most cases there will be no conflict, so the times you need to do 'git
diff' will be rather small.
Yes, many times I would like an idiom that would just replace
something with the current branch, like your A/@, but I don't know
where the right place for that would be.
Also, I feel we are missing some kind of branch, like a
remote-specific upstream, so instead of 'git reset A/foo' you would do
'git reset A@{u}'. By default the remote-specific upstream would be
the same name of the branch, but it could be configured.
Moreover, we should probably have common aliases distributed (e.g. git co).
--
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[3]: [PATCH 4/5] git-svn: fix bottleneck in stash_placeholder_list()
EW Ilya Basin basini...@gmail.com wrote: Hi. I won't send you updated patches until I import and test my huge repo. Everything will be here: https://github.com/basinilya/git/commits/v1.8.2.2-git-svn-fixes At the moment I've decided not to implement the Junio's proposal: JCH comment line # added by git-svn only to keep the directory and JCH consider a directory that has nothing but .gitignore that consists JCH of only that exact comment line an added placeholder directory to JCH work it around. But the config file is not an option too: I have 400 tags, each has 200 empty folders. Instead I decided to store the paths in a text file (see https://github.com/basinilya/git/commit/a961aedd81cb8676a52cfe71ccb6eba0f9e64b90 ). I'm not planning to push this change to you. The last error I encountered is: r7009 = 39805bb078983e34f2fc8d2c8c02d695d00d11c0 (refs/remotes/DMC4_Basic) Too many open files: Can't open file '/home/il/builds/sicap/gitsvn/prd_dmc4.svn/db/revs/0/786': Too many open files at /.snapshots/persist/builds/git/git-git/perl/blib/lib/Git/SVN/Ra.pm line 282. I think It's unrelated to empty dirs. EW Can you get an lsof on the git-svn process right before this? IB /.snapshots/persist/builds/sicap/gitsvn/aaa/.git/A4O_OTQxWc IB /.snapshots/persist/builds/sicap/gitsvn/aaa/.git/LfpcENJduN IB /.snapshots/persist/builds/sicap/gitsvn/aaa/.git/Dkk7pN4Mpz IB etc. EW What's your open files limit? IB 1024 Why no call to close() from temp_release() in Git.pm? -- -- 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: Another use of @?
On Fri, May 3, 2013 at 4:38 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Duy Nguyen pclo...@gmail.com writes:
My setup is a bit peculiar where I do git development on three
different machines. Say I updated branch long-branch-name on machine
A. Then I continue my work on machine B. I would want to hard reset
that long-branch-name on machine B before resuming my work. What I
usually do is
git co long-branch-name
git diff A/long-branch-name
git reset --hard A/long-branch-name
Perhaps
git checkout long-braTAB
git diff A/!$
git reset --hard !$
diff does not have to follow checkout.
In any case, not a Git question, I would have to say.
As a Git question, probably the answers are
git co long-braTAB
git diff @{u}
git reset --hard @{u}
with an appropriate setting of the upstream, perhaps?
and @{u} can't be used because I might want to resume from machine C
instead of A. I don't have a single upstream.
--
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
Re: [PATCH v2 4/5] index-pack, unpack-objects: add --not-so-strict for connectivity check
Junio C Hamano gits...@pobox.com writes: Duy Nguyen pclo...@gmail.com writes: What do you mean by partial history? Do we have dangling pointers after doing that commit walker? ^C will leave the objects and it is safe because it will not update refs. But your code that does not verify the full connectivity from such an object (that is outside the transferred pack) to the rest of the history will then make the resulting repository broken, if you update your ref to point at such an object, no? Ading a single has_sha1_file() only verifies that single object, not the history behind it. Let's illustrate. Imagine your project as a whole has this history: ---A---B---C---D---E When you cloned, it only had up to A, so that is what you have. Then you try http walker, which slurps E, wants to go to D and dig down, but after fetching E, some trees and blobs in E, and fetching D, but before completing D' trees and blobs, your ISP cuts you off. You have these in your object store: ---A D---E but your ref is pointing at A, because we promise that we make sure full connectivity before we update the ref, and even if you have commits D and E, the associated trees, blobs, and commits behind this subpart of the history are missing. Now you try to fetch from another mirror over the pack transport. You advertise that you have A (but you do not advertise E, because your refs do not point at it), and you expect all objects that are listed in rev-list --objects A..E to be gien to you. Unfortunately, the mirror is broken, in that it only packs the objects that appear in rev-list --objects A..B, but still tells you that it is sending objects to complete history leading to E. Your object store would have objects like this after this transfer: ---A---B D---E You may still have commits D and E unpruned, but are missing C, and trees and blobs that are needed in D or E. You have to walk the history from E and list the necessary objects yourself, running has_sha1_file() on all of them, to prove that you have everything needed, and the only things you can trust are your refs (in this case, A). If you verify that all the objects in the transferred pack are complete, and also verify that the object the transfer proposes to update your ref to is _in_ that pack, then you can detect a mirror that is broken and only sends objects for A..B, but that does not actually solve the issue. Imagine another broken mirror that instead sends objects for E. E, its trees and all its blobs may be in the pack and the only outgoing link from that pack may be a pointer out of E pointing at D. You happen to _have_ it in your object store, but that does not mean you can update your ref to E (remember, you do not have trees and blobs to complete D, or the history behind it). The current check is implemented in the way we currently do it, _not_ because we were stupid and did not realize the optimization possibility (in other words, what your patch proposes is not new), but because we rejected this approach because it was not safe. -- 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 4/5] index-pack, unpack-objects: add --not-so-strict for connectivity check
On Thu, May 02, 2013 at 11:55:57PM -0700, Junio C Hamano wrote:
Let's illustrate. Imagine your project as a whole has this history:
[snip]
OK I agree my approach is flawed. But if all these are met, we can be
sure the new refs are good, correct?
- all refs point to an object in the pack
- there are no dangling pointers in any object in the pack
- all objects in the pack does not point to any objects outside the
pack
We can check all these for the clone case with the patch below, even
if somehow we get some bad objects in the cloned repository before the
new pack arrives.
-- 8 --
Subject: [PATCH] clone: open a shortcut for connectivity check
In order to make sure the cloned repository is good, we run rev-list
--objects --not --all $new_refs on the repository. This is expensive
on large repositories. This patch attempts to mitigate the impact in
this special case.
In the good clone case, we only have one pack. If all of the
following are met, we can be sure that all objects reachable from the
new refs exist, which is the intention of running rev-list ...:
- all refs point to an object in the pack
- there are no dangling pointers in any object in the pack
- all objects in the pack does not point to objects outside the pack
The second and third checks can be done with the help of index-pack as
a slight modification of --strict check (which introduces a new
condition for the shortcut: pack transfer must be used and the number
of objects large enough to call index-pack). The first is checked in
check_everything_connected after we get an ok from index-pack.
index-pack + new checks is still faster than index-pack + rev-list
currently, which is the whole point of this patch. If any of the
conditions fails, we fall back to good old and expensive rev-list
... In that case it's even more expensive because we have to pay for
the new checks in index-pack.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-index-pack.txt | 3 +++
builtin/clone.c | 11 ---
builtin/index-pack.c | 35 ++-
connected.c | 34 +-
connected.h | 5 +
fetch-pack.c | 11 ++-
fetch-pack.h | 4 +++-
transport.c | 4
transport.h | 2 ++
9 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index bde8eec..7a4e055 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -74,6 +74,9 @@ OPTIONS
--strict::
Die, if the pack contains broken objects or links.
+--check-self-contained-and-connected::
+ Die if the pack contains broken links. For internal use only.
+
--threads=n::
Specifies the number of threads to spawn when resolving
deltas. This requires that index-pack be compiled with
diff --git a/builtin/clone.c b/builtin/clone.c
index dad4265..427bec4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -542,13 +542,15 @@ static void update_remote_refs(const struct ref *refs,
const struct ref *mapped_refs,
const struct ref *remote_head_points_at,
const char *branch_top,
- const char *msg)
+ const char *msg,
+ struct transport *transport)
{
const struct ref *rm = mapped_refs;
if (0 = option_verbosity)
printf(_(Checking connectivity... ));
- if (check_everything_connected(iterate_ref_map, 0, rm))
+ if (check_everything_connected_with_transport(iterate_ref_map,
+ 0, rm, transport))
die(_(remote did not send all necessary objects));
if (0 = option_verbosity)
printf(_(done\n));
@@ -893,6 +895,9 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
+
+ if (transport-smart_options)
+
transport-smart_options-check_self_contained_and_connected = 1;
}
refs = transport_get_remote_refs(transport);
@@ -954,7 +959,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
transport_fetch_refs(transport, mapped_refs);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
- branch_top.buf, reflog_msg.buf);
+ branch_top.buf, reflog_msg.buf, transport);
update_head(our_head_points_at, remote_head, reflog_msg.buf);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index
Information Towards Level of popularity for pandora jewerly
Earliest attention for Pandora, you may realise with regards to the wonder * pandora jewelry http://www.cheappandorausshop.net/ * ? Good, such as the glistening brightness with your girlfriend, Pandora earrings at the same time does well everyone and the great approximately everyone. Wouldn't have an individual's range of Pandora necklaces yet still? Good, that you're omitted whatever is certainly increasingly popular as of late among the most women. When you need any appreciation of everyone approximately you cannot afford to pay for reduce. *http://www.cheappandorausshop.net/ * -- View this message in context: http://git.661346.n2.nabble.com/Information-Towards-Level-of-popularity-for-pandora-jewerly-tp7584828.html Sent from the git mailing list archive at Nabble.com. -- 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
thomas sabo uk -- Name brand Revealed Perfectly By using Classy Rings
* thomas sabo charms http://www.genuinethomassaboringsshop.co.uk/ * for a make really likes a advantage to be revealed perfectly by using excellent plus really sophisticated jewellery. This is certainly mainly real by using charms plus silver products and solutions. Products and solutions offered by these folks will be famous with regard to their level of quality plus with regard to their fabulous explaining improve a treasure types. * http://www.genuinethomassaboringsshop.co.uk/ * -- View this message in context: http://git.661346.n2.nabble.com/thomas-sabo-uk-Name-brand-Revealed-Perfectly-By-using-Classy-Rings-tp7584829.html Sent from the git mailing list archive at Nabble.com. -- 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 4/5] index-pack, unpack-objects: add --not-so-strict for connectivity check
On Fri, May 3, 2013 at 3:09 AM, Duy Nguyen pclo...@gmail.com wrote: Subject: [PATCH] clone: open a shortcut for connectivity check In order to make sure the cloned repository is good, we run rev-list --objects --not --all $new_refs on the repository. This is expensive on large repositories. This patch attempts to mitigate the impact in this special case. In the good clone case, we only have one pack. If all of the following are met, we can be sure that all objects reachable from the new refs exist, which is the intention of running rev-list ...: - all refs point to an object in the pack - there are no dangling pointers in any object in the pack - all objects in the pack does not point to objects outside the pack no objects in the pack point to... The second and third checks can be done with the help of index-pack as a slight modification of --strict check (which introduces a new condition for the shortcut: pack transfer must be used and the number of objects large enough to call index-pack). The first is checked in check_everything_connected after we get an ok from index-pack. index-pack + new checks is still faster than index-pack + rev-list currently, which is the whole point of this patch. If any of the conditions fails, we fall back to good old and expensive rev-list ... In that case it's even more expensive because we have to pay for the new checks in index-pack. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- 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 2/2] cygwin: Remove the CYGWIN_V15_WIN32API build variable
On Thu, May 2, 2013 at 3:29 PM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Commit 380a4d92 (Update cygwin.c for new mingw-64 win32 api headers, 11-11-2012) solved an header include order problem on cygwin 1.7 when using the new mingw-64 WIN32 API headers. The solution involved using a new build variable (V15_MINGW_HEADERS) to conditionally compile the cygwin.c source file to use an include order appropriate for the old and new header files. (The build variable was later renamed in commit 9fca6cff to CYGWIN_V15_WIN32API). The include order used for cygwin 1.7 includes the win32.h header before ../git-compat-util.h. This order was problematic on cygwin 1.5, since it lead to the WIN32 symbol being defined along with the s/lead/led/ inclusion of some WIN32 API headers (e.g. winsock2.h) which cause compilation errors. The header include order problem on cygwin 1.5 has since been fixed (see commit mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE), so we can now remove the conditional compilation along with the associated CYGWIN_V15_WIN32API build variable. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk -- 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
Astounding Jewellery Manufactured from pandora jewellery uk
* pandora uk http://www.pandoracharmsvipsale.co.uk/ * building start out outside around Denmark together with the human being labeled Enevoldsen. Enevoldsen appeared to be your goldsmith plus your dog as well as girlfriend viewed as considering and even promotion rings manufactured from drops and charms. Now is the foundation pertaining to Pandora rings building. * http://www.pandoracharmsvipsale.co.uk/ * -- View this message in context: http://git.661346.n2.nabble.com/Astounding-Jewellery-Manufactured-from-pandora-jewellery-uk-tp7584832.html Sent from the git mailing list archive at Nabble.com. -- 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
another packed-refs race
I found another race related to the packed-refs code. Consider for a moment what happens when we are looking at refs and another process does a simultaneous git pack-refs --all --prune, updating packed-refs and deleting the loose refs. If we are resolving a single ref, then we will either find its loose form or not. If we do, we're done. If not, we can fall back on what was in the packed-refs file. If we read the packed-refs file at that point, we're OK. If the loose ref existed before but was pruned before we could read it, then we know the packed-refs file is already in place, because pack-refs would not have deleted the loose ref until it had finished writing the new file. But imagine that we already loaded the packed-refs file into memory earlier. We may be looking at an _old_ version of it that has an irrelevant sha1 from the older packed-refs file. Or it might not even exist in the packed-refs file at all, and we think the ref does not resolve. We could fix this by making sure our packed-refs file is up to date before using it. E.g., resolving a ref with the following sequence: 1. Look for loose ref. If found, OK. 2. Compare inode/size/mtime/etc of on-disk packed-refs to their values from the last time we loaded it. If they're not the same, reload packed-refs from disk. Otherwise, continue. 3. Look for ref in in-memory packed-refs. Not too bad. We introduce one extra stat() for a ref that has been packed, and the scheme isn't very complicated. But what about enumerating refs via for_each_ref? It's possible to have the same problem there, and the packed-refs file may be moved into place midway through the process of enumerating the loose refs. So we may see refs/heads/master, but when we get to refs/remotes/origin/master, it has now been packed and pruned. I _think_ we can get by with: 1. Generate the complete sorted list of loose refs. 2. Check that packed-refs is stat-clean, and reload if necessary, as above. 3. Merge the sorted loose and packed lists, letting loose override packed (so even if we get repacked halfway through our loose traversal and get half of the refs there, it's OK to see duplicates in packed-refs, which we will ignore). This is not very far off of what we do now. Obviously we don't do the stat-clean check in step 2. But we also don't generate the complete list of loose refs before hitting the packed-refs file. Instead we lazily load the loose directories as needed. And of course we cache that information in memory, even though it may go out of date. I think the best we could do is keep a stat() for each individual directory we see, and check it before using the in-memory contents. That may be a lot of stats, but it's still better than actually opening each loose ref separately. So I think it's possible to fix, but I thought you might have some insight on the simplest way to fit it into the current ref code. Did I explain the problem well enough to understand? Can you think of any simpler or better solutions (or is there a case where my proposed solutions don't work?). For reference, here's a script that demonstrates the problem during enumeration (sometimes for-each-ref fails to realize that refs/heads/master exists at all): # run this in one terminal git init repo cd repo git commit --allow-empty -m foo base=`git rev-parse HEAD` while true; do # this re-creates the loose ref in .git/refs/heads/master git update-ref refs/heads/master $base # we can remove packed-refs safely, as we know that # its only value is now stale. Real git would not do # this, but we are simulating the case that master # simply wasn't included in the last packed-refs file. rm -f .git/packed-refs # and now we repack, which will create an up-to-date # packed-refs file, and then delete the loose ref git pack-refs --all --prune done # then simultaneously run this in another cd repo while true; do refs=`git for-each-ref` echo == $refs test -z $refs break done Obviously the rm -f packed-refs above is contrived, but it does simulate a real possible state. You could also do it with a packed-refs file that has an outdated value for refs/heads/master, and demonstrate that for-each-ref sees the outdated value. -Peff -- 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: Another use of @?
Duy Nguyen pclo...@gmail.com writes:
On Fri, May 3, 2013 at 4:38 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Duy Nguyen pclo...@gmail.com writes:
My setup is a bit peculiar where I do git development on three
different machines. Say I updated branch long-branch-name on machine
A. Then I continue my work on machine B. I would want to hard reset
that long-branch-name on machine B before resuming my work. What I
usually do is
git co long-branch-name
git diff A/long-branch-name
git reset --hard A/long-branch-name
Perhaps
git checkout long-braTAB
git diff A/!$
git reset --hard !$
diff does not have to follow checkout.
At least in bash with readline, you can also use M-. to cycle through
the last arguments of the previous commands.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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: another packed-refs race
On Fri, May 3, 2013 at 10:38 AM, Jeff King p...@peff.net wrote: I found another race related to the packed-refs code. Consider for a moment what happens when we are looking at refs and another process does a simultaneous git pack-refs --all --prune, updating packed-refs and deleting the loose refs. [...] We could fix this by making sure our packed-refs file is up to date before using it. E.g., resolving a ref with the following sequence: 1. Look for loose ref. If found, OK. 2. Compare inode/size/mtime/etc of on-disk packed-refs to their values from the last time we loaded it. If they're not the same, reload packed-refs from disk. Otherwise, continue. 3. Look for ref in in-memory packed-refs. Not too bad. We introduce one extra stat() for a ref that has been packed, and the scheme isn't very complicated. But what about enumerating refs via for_each_ref? It's possible to have the same problem there, and the packed-refs file may be moved into place midway through the process of enumerating the loose refs. So we may see refs/heads/master, but when we get to refs/remotes/origin/master, it has now been packed and pruned. I _think_ we can get by with: 1. Generate the complete sorted list of loose refs. 2. Check that packed-refs is stat-clean, and reload if necessary, as above. 3. Merge the sorted loose and packed lists, letting loose override packed (so even if we get repacked halfway through our loose traversal and get half of the refs there, it's OK to see duplicates in packed-refs, which we will ignore). This is not very far off of what we do now. Obviously we don't do the stat-clean check in step 2. But we also don't generate the complete list of loose refs before hitting the packed-refs file. Instead we lazily load the loose directories as needed. And of course we cache that information in memory, even though it may go out of date. I think the best we could do is keep a stat() for each individual directory we see, and check it before using the in-memory contents. That may be a lot of stats, but it's still better than actually opening each loose ref separately. So I think it's possible to fix, but I thought you might have some insight on the simplest way to fit it into the current ref code. Did I explain the problem well enough to understand? Can you think of any simpler or better solutions (or is there a case where my proposed solutions don't work?). You don't really need to be sure that packed-refs is up-to-date. You only need to make sure that don't rely on lazily loading loose refs _after_ you have loaded packed-refs. The following solution might work in both the resolve-a-single-ref and enumerating-refs case: 0. Look for ref already cached in memory. If found, OK. 1. Look for loose ref. If found, OK. 2. If not found, load all loose refs and packed-refs from disk (in that order), and store in memory for remainder of this process. Never reload packed-refs from disk (unless you also reload all loose refs first). My rationale for this approach is that if you have a packed-refs file, you will likely have fewer loose refs, so loading all of them in addition to the pack-refs file won't be that expensive. (Conversely, if you do have a lot of loose refs, you're more likely to hit #1, and not have to load all refs.) That said, my intuition on the number of loose vs. packed refs, or the relative cost of reading all loose refs might be off here... ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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: trouble on windows network share
deg d...@degel.com writes:
I'm having this same problem.
Here's one more clue that may help: The problem is dependent on the exact
type of NAS drive.
I moved from a Buffalo LS-X2.0, which worked fine, to a WD My Book Live
(MBL), which has this problem.
I don't know much more yet about why the MBL is failing, but am still poking
around, and am happy to try tests for anyone who wants to debug this.
Can you reproduce the problem under Linux (with the NAS mounted using
CIFS), or just Windows?
If it works under Linux, you could record strace logs, e.g.
echo foo test.txt
strace -f -o trace.1 git add test.txt
strace -f -o trace.2 git commit -m test
etc.
This would massively help debugging. Judging from the OP's log, the
filesystem is just broken and can't make up its mind about what files
exist, but in the strace log we could see exactly where it gives weird
answers (or that it doesn't, and thus get clues to any possible git
bugs).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 0/3] interactive git clean
Usability observations below...
On Thu, May 2, 2013 at 11:49 PM, Jiang Xin worldhello@gmail.com wrote:
The interactive git clean combines `git clean -n` and `git clean -f`
together to do safe cleaning, and has more features.
First it displays what would be removed in columns (so that you can
see them all in one screen). The user must confirm before actually
cleaning.
WARNING: The following items will be removed permanently. Press y
WARNING: to start cleaning, and press n to abort the cleaning.
WARNING: You can also enter the edit mode, and select items
WARNING: to be excluded from the cleaning.
The user intended for files to be removed when invoking git-clean,
therefore WARNING that git-clean will do what was requested explicitly
seems overkill. Along the same lines, the user asked explicitly for an
interactive session (via --interactive), hence the above paragraph is
effectively redundant since it does little more than tell the user (in
a lengthy fashion) what he already knows (that the session is
interactive). The short prompt printed after the listed files says the
same thing (more succinctly), thus this warning paragraph is
essentially superfluous.
What would be removed...What would be removed...
What would be removed...What would be removed...
Remove (yes/no/Edit) ?
For convenience, implementations traditionally allow single letter
responses (y/n/e), but this one does not. Should it?
In this confirmation dialog, the user has three choices:
* Yes: Start to do cleaning.
* No: Nothing will be deleted.
* Edit (default for the first time): Enter the edit mode.
What about the user who desires more traditional rm -i behavior in
which he is prompted for each file? Should that be supported with a
Prompt [each] option in the above menu?
When the user chooses the edit mode, it would look like this:
NOTE: Will remove the following items. You can input space-seperated
NOTE: patterns (just like .gitignore) to exclude items from deletion,
NOTE: or press ENTER to continue.
As earlier, this (lengthy) paragraph says little more than what could
be said in a more succinct prompt printed after the file list, thus is
probably superfluous.
What would be removed...What would be removed...
What would be removed...What would be removed...
Input ignore patterns
The list of files to be removed was already shown directly above.
Dumping the entire list to the user's screen a second time upon
entering edit mode seems unnecessary. If you drop the WARNING and NOTE
paragraphs as suggested, then the session becomes much less verbose,
thus there is little reason to re-display the file list upon entering
edit mode. For instance:
% git clean -i
file1 file2 file3
file4 file5 file6
Remove (yes/no/edit)? e
Exclude (space-separated gitignore patterns): file[4-6]
file1 file2 file3
Exclude (space-separated gitignore patterns): [enter]
Remove (yes/no/edit)?
The user can input space-separated patterns (the same syntax as gitignore),
and each clean candidate that matches with one of the patterns will be
excluded from cleaning.
When the user feels it's OK, presses ENTER and back to the confirmation
dialog.
WARNING: The following items will be removed permanently. Press y
WARNING: to start cleaning, and press n to abort the cleaning.
WARNING: You can also enter the edit mode, and select items
WARNING: to be excluded from the cleaning.
What would be removed...
Remove (Yes/no/edit) ?
This time the default choice of the confirmation dialog is YES.
So when user press ENTER, start cleaning.
Is there precedent for this sort of self-mutating default action in
other utilities? Wouldn't this lead to high surprise factor for
users and potential for lost files? For instance:
% git clean -i
file1 file2 file3
file4 file5 file6
Remove (yes/no/Edit)? [enter]
{user presses ENTER for edit mode}
Exclude (space-separated gitignore patterns): file[3-6]
{user mistakenly types 3 rather than 4}
file1 file2
Exclude (space-separated gitignore patterns): [enter]
Remove (Yes/no/edit)? [enter]
{user notices mistake and presses ENTER expecting edit mode}
Removing file3
Removing file4
Removing file5
Removing file6
Oh no! The user didn't notice the subtle change of default from
yes/no/Edit to Yes/no/edit, thus he pressed ENTER thinking it
would take him to edit mode as it did initially, but instead git-clean
proceeded with the removals and file3 is lost.
Other considerations:
Is it necessary to force the user to escape from edit mode by pressing
ENTER (i.e. empty input)? Wouldn't you achieve the same level of
functionality by exiting back to the (yes/no/edit) prompt
automatically after the user enters his gitignore pattern(s)? For
instance:
% git clean -i
file1 file2 file3
file4 file5 file6
Remove (yes/no/edit)? e
Exclude (space-separated
[PATCH v3 0/4] check_everything_connected replacement
v3 is more like check_everything_connected's special case for clone because check_everything_connected is not really replaced. GIT_SHALLOW_FILE in 2/4 is now replaced by --shallow-file to avoid unintended propagation to child processes. Nguyễn Thái Ngọc Duy (4): clone: let the user know when check_everything_connected is run fetch-pack: prepare updated shallow file before fetching the pack index-pack: remove dead code (it should never happen) clone: open a shortcut for connectivity check Documentation/git-index-pack.txt | 3 ++ builtin/clone.c | 15 ++-- builtin/index-pack.c | 38 +-- commit.h | 2 + connected.c | 34 - connected.h | 5 +++ fetch-pack.c | 80 ++-- fetch-pack.h | 4 +- git.c| 5 +++ shallow.c| 45 +- t/t5500-fetch-pack.sh| 7 transport.c | 4 ++ transport.h | 2 + 13 files changed, 190 insertions(+), 54 deletions(-) -- 1.8.2.83.gc99314b -- 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 v3 1/4] clone: let the user know when check_everything_connected is run
check_everything_connected could take a long time, especially in the
clone case where the whole DAG is traversed. The user deserves to know
what's going on.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/clone.c | 4
1 file changed, 4 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index 035ab64..dad4265 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -546,8 +546,12 @@ static void update_remote_refs(const struct ref *refs,
{
const struct ref *rm = mapped_refs;
+ if (0 = option_verbosity)
+ printf(_(Checking connectivity... ));
if (check_everything_connected(iterate_ref_map, 0, rm))
die(_(remote did not send all necessary objects));
+ if (0 = option_verbosity)
+ printf(_(done\n));
if (refs) {
write_remote_refs(mapped_refs);
--
1.8.2.83.gc99314b
--
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 v3 2/4] fetch-pack: prepare updated shallow file before fetching the pack
index-pack --strict looks up and follows parent commits. If shallow
information is not ready by the time index-pack is run, index-pack may
be lead to non-existent objects. Make fetch-pack save shallow file to
disk before invoking index-pack.
git learns new global option --shallow-file to pass on the alternate
shallow file path. Undocumented (and not even support --shallow-file=
syntax) because it's unlikely to be used again elsewhere.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
commit.h | 2 ++
fetch-pack.c | 69 +--
git.c | 5
shallow.c | 45 +++--
t/t5500-fetch-pack.sh | 7 ++
5 files changed, 91 insertions(+), 37 deletions(-)
diff --git a/commit.h b/commit.h
index 67bd509..6e9c7cd 100644
--- a/commit.h
+++ b/commit.h
@@ -176,6 +176,8 @@ extern int for_each_commit_graft(each_commit_graft_fn, void
*);
extern int is_repository_shallow(void);
extern struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
+extern void check_shallow_file_for_update(void);
+extern void set_alternate_shallow_file(const char *path);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/fetch-pack.c b/fetch-pack.c
index f156dd4..1ca4f6b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -20,6 +20,8 @@ static int no_done;
static int fetch_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static int agent_supported;
+static struct lock_file shallow_lock;
+static const char *alternate_shallow_file;
#define COMPLETE (1U 0)
#define COMMON (1U 1)
@@ -683,7 +685,7 @@ static int get_pack(struct fetch_pack_args *args,
int xd[2], char **pack_lockfile)
{
struct async demux;
- const char *argv[20];
+ const char *argv[22];
char keep_arg[256];
char hdr_arg[256];
const char **av;
@@ -724,6 +726,11 @@ static int get_pack(struct fetch_pack_args *args,
do_keep = 1;
}
+ if (alternate_shallow_file) {
+ *av++ = --shallow-file;
+ *av++ = alternate_shallow_file;
+ }
+
if (do_keep) {
if (pack_lockfile)
cmd.out = -1;
@@ -779,6 +786,23 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
return strcmp(a-name, b-name);
}
+static void setup_alternate_shallow(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+
+ check_shallow_file_for_update();
+ fd = hold_lock_file_for_update(shallow_lock, git_path(shallow),
+ LOCK_DIE_ON_ERROR);
+ if (write_shallow_commits(sb, 0)) {
+ if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+ die_errno(failed to write to %s,
shallow_lock.filename);
+ alternate_shallow_file = shallow_lock.filename;
+ } else
+ alternate_shallow_file = ;
+ strbuf_release(sb);
+}
+
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
@@ -858,6 +882,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
if (args-stateless_rpc)
packet_flush(fd[1]);
+ if (args-depth 0)
+ setup_alternate_shallow();
if (get_pack(args, fd, pack_lockfile))
die(git fetch-pack: fetch failed.);
@@ -936,15 +962,9 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
struct ref **sought, int nr_sought,
char **pack_lockfile)
{
- struct stat st;
struct ref *ref_cpy;
fetch_pack_setup();
- if (args-depth 0) {
- if (stat(git_path(shallow), st))
- st.st_mtime = 0;
- }
-
if (nr_sought)
nr_sought = remove_duplicates_in_refs(sought, nr_sought);
@@ -955,34 +975,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
pack_lockfile);
if (args-depth 0) {
- static struct lock_file lock;
- struct cache_time mtime;
- struct strbuf sb = STRBUF_INIT;
- char *shallow = git_path(shallow);
- int fd;
-
- mtime.sec = st.st_mtime;
- mtime.nsec = ST_MTIME_NSEC(st);
- if (stat(shallow, st)) {
- if (mtime.sec)
- die(shallow file was removed during fetch);
- } else if (st.st_mtime != mtime.sec
-#ifdef USE_NSEC
- || ST_MTIME_NSEC(st) != mtime.nsec
-#endif
- )
-
[PATCH v3 4/4] clone: open a shortcut for connectivity check
In order to make sure the cloned repository is good, we run rev-list
--objects --not --all $new_refs on the repository. This is expensive
on large repositories. This patch attempts to mitigate the impact in
this special case.
In the good clone case, we only have one pack. If all of the
following are met, we can be sure that all objects reachable from the
new refs exist, which is the intention of running rev-list ...:
- all refs point to an object in the pack
- there are no dangling pointers in any object in the pack
- no objects in the pack point to objects outside the pack
The second and third checks can be done with the help of index-pack as
a slight variation of --strict check (which introduces a new condition
for the shortcut: pack transfer must be used and the number of objects
large enough to call index-pack). The first is checked in
check_everything_connected after we get an ok from index-pack.
index-pack + new checks is still faster than the current index-pack
+ rev-list, which is the whole point of this patch. If any of the
conditions fails, we fall back to the good old but expensive rev-list
... In that case it's even more expensive because we have to pay for
the new checks in index-pack. But that should only happen when the
other side is either buggy or malicious.
Cloning linux-2.6 over file://
before after
real3m25.693s 2m53.050s
user5m2.037s 4m42.396s
sys 0m13.750s 0m16.574s
A more realistic test with ssh:// over wireless
before after
real11m26.629s 10m4.213s
user5m43.196s 5m19.444s
sys 0m35.812s 0m37.630s
This shortcut is not applied to shallow clones, partly because shallow
clones should have no more objects than a usual fetch and the cost of
rev-list is acceptable, partly to avoid dealing with corner cases when
grafting is involved.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-index-pack.txt | 3 +++
builtin/clone.c | 11 ---
builtin/index-pack.c | 35 ++-
connected.c | 34 +-
connected.h | 5 +
fetch-pack.c | 11 ++-
fetch-pack.h | 4 +++-
transport.c | 4
transport.h | 2 ++
9 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index bde8eec..7a4e055 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -74,6 +74,9 @@ OPTIONS
--strict::
Die, if the pack contains broken objects or links.
+--check-self-contained-and-connected::
+ Die if the pack contains broken links. For internal use only.
+
--threads=n::
Specifies the number of threads to spawn when resolving
deltas. This requires that index-pack be compiled with
diff --git a/builtin/clone.c b/builtin/clone.c
index dad4265..069e81e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -542,13 +542,15 @@ static void update_remote_refs(const struct ref *refs,
const struct ref *mapped_refs,
const struct ref *remote_head_points_at,
const char *branch_top,
- const char *msg)
+ const char *msg,
+ struct transport *transport)
{
const struct ref *rm = mapped_refs;
if (0 = option_verbosity)
printf(_(Checking connectivity... ));
- if (check_everything_connected(iterate_ref_map, 0, rm))
+ if (check_everything_connected_with_transport(iterate_ref_map,
+ 0, rm, transport))
die(_(remote did not send all necessary objects));
if (0 = option_verbosity)
printf(_(done\n));
@@ -893,6 +895,9 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
+
+ if (transport-smart_options !option_depth)
+
transport-smart_options-check_self_contained_and_connected = 1;
}
refs = transport_get_remote_refs(transport);
@@ -954,7 +959,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
transport_fetch_refs(transport, mapped_refs);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
- branch_top.buf, reflog_msg.buf);
+ branch_top.buf, reflog_msg.buf, transport);
update_head(our_head_points_at, remote_head, reflog_msg.buf);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index
Re: [PATCH v3 2/4] fetch-pack: prepare updated shallow file before fetching the pack
On Fri, May 3, 2013 at 8:35 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: index-pack --strict looks up and follows parent commits. If shallow information is not ready by the time index-pack is run, index-pack may be lead to non-existent objects. Make fetch-pack save shallow file to s/lead/led/ disk before invoking index-pack. git learns new global option --shallow-file to pass on the alternate shallow file path. Undocumented (and not even support --shallow-file= syntax) because it's unlikely to be used again elsewhere. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- 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 v3 4/4] clone: open a shortcut for connectivity check
On Fri, May 3, 2013 at 8:35 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: In order to make sure the cloned repository is good, we run rev-list --objects --not --all $new_refs on the repository. This is expensive on large repositories. This patch attempts to mitigate the impact in this special case. In the good clone case, we only have one pack. If all of the following are met, we can be sure that all objects reachable from the new refs exist, which is the intention of running rev-list ...: - all refs point to an object in the pack - there are no dangling pointers in any object in the pack - no objects in the pack point to objects outside the pack The second and third checks can be done with the help of index-pack as a slight variation of --strict check (which introduces a new condition for the shortcut: pack transfer must be used and the number of objects large enough to call index-pack). The first is checked in check_everything_connected after we get an ok from index-pack. index-pack + new checks is still faster than the current index-pack + rev-list, which is the whole point of this patch. If any of the conditions fails, we fall back to the good old but expensive rev-list s/fails/fail/ ... In that case it's even more expensive because we have to pay for the new checks in index-pack. But that should only happen when the other side is either buggy or malicious. Cloning linux-2.6 over file:// before after real3m25.693s 2m53.050s user5m2.037s 4m42.396s sys 0m13.750s 0m16.574s A more realistic test with ssh:// over wireless before after real11m26.629s 10m4.213s user5m43.196s 5m19.444s sys 0m35.812s 0m37.630s This shortcut is not applied to shallow clones, partly because shallow clones should have no more objects than a usual fetch and the cost of rev-list is acceptable, partly to avoid dealing with corner cases when grafting is involved. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- 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 v4 2/3] Show items of interactive git-clean in columns
Jiang Xin worldhello@gmail.com writes: Rewrite the log as following: That's probably more than needed ;-). Thanks, Show items of interactive git-clean in columns When there are lots of items to be cleaned, it is hard to see them all in one screen. Show them in columns instead of in one column will solve this problem. Since no longer show items to be cleaned using the Would remove ... Since _we_ no longer ...? Will honors column.ui config variable only, not introduce new config variable. So no more documentations needed ;-) I don't think it's a good idea. Usually, the *.ui variables are shortcuts for set all the individual variables, and having things customizeable by column.ui and not by anything else breaks this. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 0/3] interactive git clean
Jiang Xin worldhello@gmail.com writes: The interactive git clean combines `git clean -n` and `git clean -f` together to do safe cleaning, and has more features. First it displays what would be removed in columns (so that you can see them all in one screen). The user must confirm before actually cleaning. WARNING: The following items will be removed permanently. Press y WARNING: to start cleaning, and press n to abort the cleaning. WARNING: You can also enter the edit mode, and select items WARNING: to be excluded from the cleaning. What would be removed...What would be removed... What would be removed...What would be removed... To a user who explicitly _asked_ to run the interactive mode, I find these four warning lines that shout at the user in all caps way overkill. I would have expected the output to begin with a line to explain what it is listing (e.g. Cleaning the following files:), the list and then Remove (Yes/no/edit/?) ? The existing add -p prompt uses this trick to hint the user that a further help is available by typing ?, and it is a good example to follow. -- 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 v3 4/4] clone: open a shortcut for connectivity check
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: In order to make sure the cloned repository is good, we run rev-list --objects --not --all $new_refs on the repository. This is expensive on large repositories. This patch attempts to mitigate the impact in this special case. In the good clone case, we only have one pack. If On large repositories is the focus, we need to take into account the fact that pack.packSizeLimit can split and store the incoming packstream to multiple packs, so only have one pack is misleading. I think you can still do the same trick even when we split the pack as index-pack will keep track of the objects it saw in the same incoming pack stream (but I am writing this from memory without looking at the original code you are touching, so please double check). If all of the following are met, we can be sure that all objects reachable from the new refs exist, which is the intention of running rev-list ...: - all refs point to an object in the pack - there are no dangling pointers in any object in the pack - no objects in the pack point to objects outside the pack The second and third checks can be done with the help of index-pack as a slight variation of --strict check (which introduces a new condition for the shortcut: pack transfer must be used and the number of objects large enough to call index-pack). The first is checked in check_everything_connected after we get an ok from index-pack. index-pack + new checks is still faster than the current index-pack + rev-list, which is the whole point of this patch. If any of the Does the same check apply if we end up on the unpack-objects codepath? This shortcut is not applied to shallow clones, partly because shallow clones should have no more objects than a usual fetch and the cost of rev-list is acceptable, partly to avoid dealing with corner cases when grafting is involved. -- 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: another packed-refs race
On Fri, May 03, 2013 at 11:26:11AM +0200, Johan Herland wrote: You don't really need to be sure that packed-refs is up-to-date. You only need to make sure that don't rely on lazily loading loose refs _after_ you have loaded packed-refs. True. As long as you load them both together, and always make sure you do loose first, you'd be fine. But I think there will be corner cases where you have loaded _part_ of the loose ref namespace. I think part of the point of Michael's ref work is that if you call for_each_tag_ref, we would not waste time loading refs/remotes/ at all. If you then follow that with a call to for_each_ref, you would want to re-use the cached work from traversing refs/tags/, and then traverse refs/remotes/. You know that your cached packed-refs is good with respect to refs/tags/, but you don't with respect to refs/remotes. The following solution might work in both the resolve-a-single-ref and enumerating-refs case: 0. Look for ref already cached in memory. If found, OK. 1. Look for loose ref. If found, OK. 2. If not found, load all loose refs and packed-refs from disk (in that order), and store in memory for remainder of this process. Never reload packed-refs from disk (unless you also reload all loose refs first). I think that would be correct (modulo that step 1 cannot happen for enumeration). But we would like to avoid loading all loose refs if we can. Especially on a cold cache, it can be quite slow, and you may not even care about those refs for the current operation (I do not recall the exact original motivation for the lazy loading, but it was something along those lines). My rationale for this approach is that if you have a packed-refs file, you will likely have fewer loose refs, so loading all of them in addition to the pack-refs file won't be that expensive. (Conversely, if you do have a lot of loose refs, you're more likely to hit #1, and not have to load all refs.) That said, my intuition on the number of loose vs. packed refs, or the relative cost of reading all loose refs might be off here... I don't think that is necessarily true about the number of loose refs. In a busy repository, you may have many loose refs that have been updated. -Peff -- 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] fast-import: only store commit objects
Felipe Contreras felipe.contre...@gmail.com writes:
There's no point in storing blob, they would increase the time of
loading the marks, and the vast majority of them will never be used
again.
This also makes fast-export and fast-import marks compatible.
[...]
- if (m-data.marked[k])
+ if (m-data.marked[k]) {
+ struct object_entry *e;
+ e = m-data.marked[k];
+ if (e-type != OBJ_COMMIT)
+ continue;
fprintf(f, :% PRIuMAX %s\n, base + k,
-
sha1_to_hex(m-data.marked[k]-idx.sha1));
+ sha1_to_hex(e-idx.sha1));
+ }
IIUC, you are unconditionally storing only marks to commit objects.
Are you allowed to do that at this point? I notice that
git-fast-export(1) says
--export-marks=file
Dumps the internal marks table to file when complete. Marks are
written one per line as :markid SHA-1. Only marks for revisions
are dumped[...]
But git-fast-import(1) says nothing of the sort; I would even claim that
--export-marks=file
Dumps the internal marks table to file when complete.
means that the *full* marks table is dumped.
How do we know that this doesn't break any users of fast-import? Your
comment isn't very reassuring:
the vast majority of them will never be used again
So what's with the minority?
In any case, if this does go in, please update the documentation to
match, probably by copying the sentence from git-fast-export(1).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] fast-import: only store commit objects
On Fri, May 3, 2013 at 12:56 PM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: How do we know that this doesn't break any users of fast-import? Your comment isn't very reassuring: the vast majority of them will never be used again So what's with the minority? Actually I don't think there's any minority. If the client program doesn't store blobs, the blob marks are not used anyway. So there's no change. However, there's a chance the client programs do rely on blob objects, in which case things would break. I would like to analyze some client programs of fast-import out there, to see what would be the impact. But I think this has to be done either way, the only question is how. Having a warning would take a lot of effort, and it might be for nothing. I think it might make sense to knowingly make the potentially dangerous change, and see what breaks. Most likely nothing will, but if something does, we revert immediately. Otherwise we would be stuck in this non-ideal state forever. Cheers. -- 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: another packed-refs race
On Fri, May 03, 2013 at 01:28:53PM -0400, Jeff King wrote: The following solution might work in both the resolve-a-single-ref and enumerating-refs case: 0. Look for ref already cached in memory. If found, OK. 1. Look for loose ref. If found, OK. 2. If not found, load all loose refs and packed-refs from disk (in that order), and store in memory for remainder of this process. Never reload packed-refs from disk (unless you also reload all loose refs first). I think that would be correct (modulo that step 1 cannot happen for enumeration). But we would like to avoid loading all loose refs if we can. Especially on a cold cache, it can be quite slow, and you may not even care about those refs for the current operation (I do not recall the exact original motivation for the lazy loading, but it was something along those lines). Actually, forgetting about enumeration for a minute, that would make single-ref lookup quite painful. Running git rev-parse foo shouldn't have to even look at most loose refs in the first place. It should be a couple of open() calls looking for the right spot, and then fall back to loading packed-refs. -Peff -- 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: submodules
Am 03.05.2013 15:45, schrieb shawn wilson: So, I actually have another question I wasn't able to get to in this example (which has color - sorry - less -F displays it decently) What is shown here is that trying to add submodules in this repo doesn't add the .gitmodules file - I can do it manually, but why isn't it adding the file? There's a .git/modules directory populated with the right stuff, but no .gitmodules file. The initial question I was trying to demonstrate was that I've got a repo with submodules. When I push branches to most of the modules, a: git branch -r shows them for everyone. However, in one repo/module (I think it's a repo created with git --bare --shared) no one else can see (or pull) the remote branches and if I make a new clone of that repo as myself, I can't see them either. However, those branches are there and if I check that repo out on its own (not as a submodule of the main repo) I and everyone else can see those remote branches. This is git 1.8.2.1 btw. Thanks for your report. Unfortunately I'm not able to see much in your attachment, could you please try to reproduce your problem with a few shell commands and send these inline? -- 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: Suggestion for improving the manual page for git submodule
Am 03.05.2013 03:23, schrieb Dale R. Worley: Several people have made similar mistakes in beliving that git submodule init can be used for adding submodules to a working directory, whereas git submodule add is the command that should be used. That *is* documented at the top of the manual page for git submodule, but my error was enhanced by a subtle error in the documentation of init. Thanks for bringing this up. The text as it is written suggests that init's behavior is driven by the contents of .submodules. But in fact, its behavior is driven by the existing gitlinks in the file tree, possibly limited by the path arguments. (Which is *why* git submodule init can't *add* submodules; it only processes *existing* submodules.) That's correct (but I think index should be used here instead of file tree). I would like to suggest that the manual page be updated to remove the error in the description of the init subcommand, along with another addition to tell the submodule logical name that is used by the add subcommand: Good idea, care to provide a patch? ;-) (If so, please see Documentation/SubmittingPatches on how to do that) --- man1 2013-04-26 12:02:16.752702146 -0400 +++ man3 2013-05-02 21:06:00.020353100 -0400 @@ -61,6 +61,8 @@ to exist in the superproject. If path is not given, the humanish part of the source repository is used (repo for /path/to/repo.git and foo for host.xz:foo/.git). + The path is used as the submodule's logical name in its + configuration entries. Nice, but I think you should append unless the --name option is used to provide a different name or such to that sentence. repository is the URL of the new submodule’s origin repository. This may be either an absolute URL, or (if it begins with ./ or @@ -109,7 +111,9 @@ too (and can also report changes to a submodule’s work tree). init - Initialize the submodules, i.e. register each submodule name and + Initialize the submodules, i.e. register each submodule for which + there is a gitlink recorded (or the specific gitlinks specified by + path ...) by copying the name and This sounds very technical ... maybe we should rephrase that like this? - Initialize the submodules, i.e. register each submodule name and + Initialize the submodules recorded in the index (by having been + added and committed somewhere else), i.e. register each submodule + name and (Not being a native speaker I would appreciate if somebody else comes up with a better wording. I think we should talk about the fact that somebody else already added this submodule in his work tree; I'm not sure talking about a gitlink here would help someone new to submodules that much, as this topic seems to be about confusing init and add.) url found in .gitmodules into .git/config. The key used in .git/config is submodule.$name.url. This command does not alter existing information in .git/config. You can then customize the @@ -118,6 +122,10 @@ submodule update --init without the explicit init step if you do not intend to customize any submodule locations. + (Because init only operates on existing gitlinks, it cannot + be used to create submodules, regardless of the contents of + .gitmodules. Use the add subcommand to create submodules.) + I'm not sure we need this anymore when we clarify the description above. update Update the registered submodules, i.e. clone missing submodules and checkout the commit specified in the index of the containing -- 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: another packed-refs race
On Fri, May 3, 2013 at 8:26 PM, Jeff King p...@peff.net wrote: On Fri, May 03, 2013 at 01:28:53PM -0400, Jeff King wrote: The following solution might work in both the resolve-a-single-ref and enumerating-refs case: 0. Look for ref already cached in memory. If found, OK. 1. Look for loose ref. If found, OK. 2. If not found, load all loose refs and packed-refs from disk (in that order), and store in memory for remainder of this process. Never reload packed-refs from disk (unless you also reload all loose refs first). I think that would be correct (modulo that step 1 cannot happen for enumeration). But we would like to avoid loading all loose refs if we can. Especially on a cold cache, it can be quite slow, and you may not even care about those refs for the current operation (I do not recall the exact original motivation for the lazy loading, but it was something along those lines). Actually, forgetting about enumeration for a minute, that would make single-ref lookup quite painful. Running git rev-parse foo shouldn't have to even look at most loose refs in the first place. It should be a couple of open() calls looking for the right spot, and then fall back to loading packed-refs. True. I was overemphasizing the case where we start looking up one ref, and later look up more refs from the same process (in which case the load-everything step would be amortized across the other lookups), but this is probably not the ref access pattern for most Git commands, and definitely not for git rev-parse foo. I think your approach is better. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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: another packed-refs race
On Fri, May 03, 2013 at 04:38:47AM -0400, Jeff King wrote:
For reference, here's a script that demonstrates the problem during
enumeration (sometimes for-each-ref fails to realize that
refs/heads/master exists at all):
# run this in one terminal
git init repo
cd repo
git commit --allow-empty -m foo
base=`git rev-parse HEAD`
while true; do
# this re-creates the loose ref in .git/refs/heads/master
git update-ref refs/heads/master $base
It turns out this is wrong. Git is smart enough not to bother writing
out the loose ref if it isn't changing. So the script as I showed it
actually ends up in a state with _neither_ the packed-refs file nor the
loose ref for an instant.
The correct script looks like this (it just flips between two objects):
git init -q repo
cd repo
git commit -q --allow-empty -m one
one=`git rev-parse HEAD`
git commit -q --allow-empty -m two
two=`git rev-parse HEAD`
sha1=$one
while true; do
# this re-creates the loose ref in .git/refs/heads/master
if test $sha1 = $one; then
sha1=$two
else
sha1=$one
fi
git update-ref refs/heads/master $sha1
# we can remove packed-refs safely, as we know that
# its only value is now stale. Real git would not do
# this, but we are simulating the case that master
# simply wasn't included in the last packed-refs file.
rm -f .git/packed-refs
# and now we repack, which will create an up-to-date
# packed-refs file, and then delete the loose ref
git pack-refs --all --prune
done
And a racy lookup check could look like this:
cd repo
while true; do
ref=`git rev-parse --verify master`
echo == $ref
test -z $ref break
done
it doesn't know which of the two flipping refs it will get on any given
invocation, but it should never see nothing. It should get one or the
other. With stock git, running these two looks for me simultaneously
typically causes a failure in the second one within about 15 seconds.
The (messy, not ready for application) patch below fixes it (at least I
let it run for 30 minutes without a problem).
The fix is actually two-fold:
1. Re-load the packed-refs file after each loose object lookup
failure. This is made more palatable by using stat() to avoid
re-reading the file in the common case that it wasn't updated.
2. The loose ref reading itself is actually not atomic. We call
lstat() on the ref to find out whether it exists (and whether it is
a symlink). If we get ENOENT, we fall back to finding the loose
ref. If it does exist and is a regular file, we proceed to open()
it. But if the ref gets packed and pruned in the interim, our open
will fail and we just return NULL to say oops, I guess it doesn't
exist. We want the same fallback-to-packed behavior we would get
if the lstat failed.
We could potentially do the same when we readlink() a symbolic
link, but I don't think it is necessary. We do not pack symbolic
refs, so if readlink gets ENOENT, it's OK to say nope, the ref
does not exist.
This doesn't cover the for_each_ref enumeration case at all, which
should still fail. I'll try to look at that next.
---
diff --git a/refs.c b/refs.c
index de2d8eb..45a7ee6 100644
--- a/refs.c
+++ b/refs.c
@@ -708,6 +708,7 @@ static struct ref_cache {
struct ref_cache *next;
struct ref_entry *loose;
struct ref_entry *packed;
+ struct stat packed_validity;
/* The submodule name, or for the main repo. */
char name[FLEX_ARRAY];
} *ref_cache;
@@ -717,6 +718,7 @@ static void clear_packed_ref_cache(struct ref_cache *refs)
if (refs-packed) {
free_ref_entry(refs-packed);
refs-packed = NULL;
+ memset(refs-packed_validity, 0,
sizeof(refs-packed_validity));
}
}
@@ -876,19 +878,57 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
}
}
+/*
+ * Returns 1 if the cached stat information matches the
+ * current state of the file, and 0 otherwise. This should
+ * probably be refactored to share code with ce_match_stat_basic,
+ * which has platform-specific knobs for which fields to respect.
+ */
+static int check_stat_validity(const struct stat *old, const char *fn)
+{
+ static struct stat null;
+ struct stat cur;
+
+ if (stat(fn, cur))
+ return errno == ENOENT !memcmp(old, null, sizeof(null));
+ return cur.st_ino == old-st_ino
+ cur.st_size == old-st_size
+ cur.st_mtime == old-st_mtime;
+}
+
+/*
+ * Call fstat, but zero out the stat structure if for whatever
+ * reason we can't get an answer.
+ */
+static int safe_fstat(int fd, struct stat *out)
+{
+ int r = fstat(fd, out);
+ if (r)
+ memset(out, 0, sizeof(*out));
+ return r;
+}
+
static struct ref_dir *get_packed_refs(struct ref_cache *refs)
{
+ const char
[PATCH] completion: zsh: don't override suffix on _detault
zsh is smart enough to add the right suffix while completing, there's no
point in trying to do the same as bash.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.zsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.zsh
b/contrib/completion/git-completion.zsh
index 49f0cb8..2565d2e 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -198,7 +198,7 @@ _git ()
emulate ksh -c __${service}_main
fi
- let _ret _default -S '' _ret=0
+ let _ret _default _ret=0
return _ret
}
--
1.8.3.rc0.401.g45bba44
--
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 1/4] fast-{import,export}: use get_sha1_hex() directly
Felipe Contreras felipe.contre...@gmail.com writes:
There's no point in calling get_sha1() if we know they are SHA-1s.
If we know they _have to be_ 40-hex object names, calling get_sha1()
is not just pointless but outright wrong and these calls have to be
get_sha1_hex().
Looks like a good change to me.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/fast-export.c | 2 +-
fast-import.c | 10 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d60d675..a4dee14 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -621,7 +621,7 @@ static void import_marks(char *input_file)
mark = strtoumax(line + 1, mark_end, 10);
if (!mark || mark_end == line + 1
- || *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
+ || *mark_end != ' ' || get_sha1_hex(mark_end + 1, sha1))
die(corrupt mark line: %s, line);
if (last_idnum mark)
diff --git a/fast-import.c b/fast-import.c
index 5f539d7..e02f212 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1822,7 +1822,7 @@ static void read_marks(void)
*end = 0;
mark = strtoumax(line + 1, end, 10);
if (!mark || end == line + 1
- || *end != ' ' || get_sha1(end + 1, sha1))
+ || *end != ' ' || get_sha1_hex(end + 1, sha1))
die(corrupt mark line: %s, line);
e = find_object(sha1);
if (!e) {
@@ -2490,7 +2490,7 @@ static void note_change_n(struct branch *b, unsigned
char *old_fanout)
if (commit_oe-type != OBJ_COMMIT)
die(Mark :% PRIuMAX not a commit, commit_mark);
hashcpy(commit_sha1, commit_oe-idx.sha1);
- } else if (!get_sha1(p, commit_sha1)) {
+ } else if (!get_sha1_hex(p, commit_sha1)) {
unsigned long size;
char *buf = read_object_with_reference(commit_sha1,
commit_type, size, commit_sha1);
@@ -2604,7 +2604,7 @@ static int parse_from(struct branch *b)
free(buf);
} else
parse_from_existing(b);
- } else if (!get_sha1(from, b-sha1))
+ } else if (!get_sha1_hex(from, b-sha1))
parse_from_existing(b);
else
die(Invalid ref name or SHA1 expression: %s, from);
@@ -2632,7 +2632,7 @@ static struct hash_list *parse_merge(unsigned int
*count)
if (oe-type != OBJ_COMMIT)
die(Mark :% PRIuMAX not a commit, idnum);
hashcpy(n-sha1, oe-idx.sha1);
- } else if (!get_sha1(from, n-sha1)) {
+ } else if (!get_sha1_hex(from, n-sha1)) {
unsigned long size;
char *buf = read_object_with_reference(n-sha1,
commit_type, size, n-sha1);
@@ -2792,7 +2792,7 @@ static void parse_new_tag(void)
oe = find_mark(from_mark);
type = oe-type;
hashcpy(sha1, oe-idx.sha1);
- } else if (!get_sha1(from, sha1)) {
+ } else if (!get_sha1_hex(from, sha1)) {
struct object_entry *oe = find_object(sha1);
if (!oe) {
type = sha1_object_info(sha1, NULL);
--
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/4] fast-export: improve speed by skipping blobs
Felipe Contreras felipe.contre...@gmail.com writes: We don't care about blobs, or any object other than commits, but in order to find the type of object, we are parsing the whole thing, which is slow, specially in big repositories with lots of big files. There's no need for that, we can query the object information with sha1_object_info(); Before this, loading the objects of a fresh emacs import, with 260598 blobs took 14 minutes, after this patch, it takes 3 seconds. OK. This is the way fast-import does it. Also die if the object is not found (like fast-import). Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/fast-export.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index a4dee14..a5b8da8 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -613,6 +613,7 @@ static void import_marks(char *input_file) char *line_end, *mark_end; unsigned char sha1[20]; struct object *object; + enum object_type type; line_end = strchr(line, '\n'); if (line[0] != ':' || !line_end) @@ -627,17 +628,19 @@ static void import_marks(char *input_file) if (last_idnum mark) last_idnum = mark; - object = parse_object(sha1); - if (!object) + type = sha1_object_info(sha1, NULL); + if (type 0) + die(object not found: %s, sha1_to_hex(sha1)); + + if (type != OBJ_COMMIT) + /* only commits */ continue; + object = parse_object(sha1); + if (object-flags SHOWN) error(Object %s already has a mark, sha1_to_hex(sha1)); - if (object-type != OBJ_COMMIT) - /* only commits */ - continue; - mark_object(object, mark); object-flags |= SHOWN; -- 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 3/4] fast-export: don't parse all the commits
Felipe Contreras felipe.contre...@gmail.com writes: We don't need the parsed objects at this point, merely the information that they have marks. Seems to be three times faster in my setup with lots of objects. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/fast-export.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index a5b8da8..3c5a701 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -636,7 +636,7 @@ static void import_marks(char *input_file) /* only commits */ continue; - object = parse_object(sha1); + object = lookup_unknown_object(sha1); This updates the parse_object() moved by the previous patch. At this point in the codeflow, unlike the original, we already _know_ the object must be a commit; wouldn't an equivalent of: object = (lookup_commit(sha1)-object) be more correct here? if (object-flags SHOWN) error(Object %s already has a mark, sha1_to_hex(sha1)); -- 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] fast-import: only store commit objects
Thomas Rast tr...@inf.ethz.ch writes: IIUC, you are unconditionally storing only marks to commit objects. Are you allowed to do that at this point? I notice that git-fast-export(1) says --export-marks=file Dumps the internal marks table to file when complete. Marks are written one per line as :markid SHA-1. Only marks for revisions are dumped[...] But git-fast-import(1) says nothing of the sort; I would even claim that --export-marks=file Dumps the internal marks table to file when complete. means that the *full* marks table is dumped. How do we know that this doesn't break any users of fast-import? Your comment isn't very reassuring: the vast majority of them will never be used again So what's with the minority? In any case, if this does go in, please update the documentation to match, probably by copying the sentence from git-fast-export(1). A safe and sane approach may be to teach these an option to tell them to omit non-commits or to emit all kinds, and make remote-bzr use that to exclude non-commits. If the defaults is matched to the current behaviour, nobody will get hurt, and Felipe's Emacs import, knowing that it does not need marks to blobs, can take advantage of the new feature. -- 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: Another use of @?
Junio C Hamano gits...@pobox.com writes:
Duy Nguyen pclo...@gmail.com writes:
My setup is a bit peculiar where I do git development on three
different machines. Say I updated branch long-branch-name on machine
A. Then I continue my work on machine B. I would want to hard reset
that long-branch-name on machine B before resuming my work. What I
usually do is
git co long-branch-name
git diff A/long-branch-name
git reset --hard A/long-branch-name
Perhaps
git checkout long-braTAB
git diff A/!$
git reset --hard !$
I think Duy meant
git diff A/$(git symbolic-ref --short HEAD)
i.e. the branch with the same name as the current one, but on a
different remote. If this is the question, then it is a Git thing more
than a shell one.
The A/@ could make sense, but I'm wondering whether we're taking the
direction of implementing some kind of Brainfuck dialect in Git revision
specifiers. I'm not sure we want to add more special characters here and
there with subtly different meanings (@ = HEAD, @{1} = HEAD@{1}, A/@ =
A/$(git symbolic-ref --short HEAD)).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] fast-import: only store commit objects
On Fri, May 3, 2013 at 5:08 PM, Junio C Hamano gits...@pobox.com wrote: Thomas Rast tr...@inf.ethz.ch writes: IIUC, you are unconditionally storing only marks to commit objects. Are you allowed to do that at this point? I notice that git-fast-export(1) says --export-marks=file Dumps the internal marks table to file when complete. Marks are written one per line as :markid SHA-1. Only marks for revisions are dumped[...] But git-fast-import(1) says nothing of the sort; I would even claim that --export-marks=file Dumps the internal marks table to file when complete. means that the *full* marks table is dumped. How do we know that this doesn't break any users of fast-import? Your comment isn't very reassuring: the vast majority of them will never be used again So what's with the minority? In any case, if this does go in, please update the documentation to match, probably by copying the sentence from git-fast-export(1). A safe and sane approach may be to teach these an option to tell them to omit non-commits or to emit all kinds, and make remote-bzr use that to exclude non-commits. This has nothing to do with remote-bzr, or any remote helper. These objects are not useful, not even to 'git fast-export'. If the defaults is matched to the current behaviour, nobody will get hurt Changing nothing always ensures that nobody will get hurt, but that doesn't improve anything either. -- 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
Pitfalls in auto-fast-forwarding heads that are not checked out?
I am building a small git wrapper around puppet, and one of the actions it performs is auto-fastforwarding of branches without checking them out. In simplified code... we ensure that we are on a head called master, and in some cases ppg commit, will commit to master and... ## early on # sanity-check we are on master headname=$(git rev-parse --symbolic-full-name --revs-only HEAD) if [ $headname -ne refs/heads/headname ]; then echo 2 ERROR: can only issue --immediate commit from the master branch! exit 1 fi ## then git commit -bla blarg baz ## and then... # ensure we can ff head_sha1=$(git rev-parse --revs-only master) mb=$(git merge-base $production_sha1 refs/heads/master) if [[ $mb -ne $production_sha1 ]]; then echo 2 ERROR: cannot fast-forward master to production exit 1 fi $GIT_EXEC_PATH/git-update-ref -m ppg immediate commit refs/heads/production $head_sha1 $production_sha1 || exit 1 Are there major pitfalls in this approach? I cannot think of any, but git has stayed away from updating my local tracking branches; so maybe there's a reason for that... cheers, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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] git-svn: added an --include-path flag
The SVN::Fetcher module is now able to filter for inclusion as well
as exclusion (as used by --ignore-path). Also added tests, documentation
changes and git completion script.
If you have an SVN repository with many top level directories and you
only want a git-svn clone of some of them then using --ignore-path is
difficult as it requires a very long regexp. In this case it's much
easier to filter for inclusion.
Signed-off-by: Paul Walmsley pjwh...@gmail.com
---
Documentation/git-svn.txt | 12 +++
contrib/completion/git-completion.bash |2 +-
git-svn.perl |4 +
perl/Git/SVN/Fetcher.pm| 16 +++-
t/t9147-git-svn-include-paths.sh | 149
5 files changed, 180 insertions(+), 3 deletions(-)
create mode 100755 t/t9147-git-svn-include-paths.sh
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1b8b649..698a6bb 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -85,6 +85,10 @@ COMMANDS
When passed to 'init' or 'clone' this regular expression will
be preserved as a config key. See 'fetch' for a description
of '--ignore-paths'.
+--include-paths=regex;;
+ When passed to 'init' or 'clone' this regular expression will
+ be preserved as a config key. See 'fetch' for a description
+ of '--include-paths'.
--no-minimize-url;;
When tracking multiple directories (using --stdlayout,
--branches, or --tags options), git svn will attempt to connect
@@ -146,6 +150,14 @@ Skip branches and tags of first level directories;;
--
+--include-paths=regex;;
+ This allows one to specify a Perl regular expression that will
+ cause the inclusion of only matching paths from checkout from SVN.
+ The '--include-paths' option should match for every 'fetch'
+ (including automatic fetches due to 'clone', 'dcommit',
+ 'rebase', etc) on a given repository. '--ignore-paths' takes
+ precedence over '--include-paths'.
+
--log-window-size=n;;
Fetch n log entries per request when scanning Subversion history.
The default is 100. For very large Subversion repositories, larger
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 2ba1461..8427df2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2451,7 +2451,7 @@ _git_svn ()
--no-metadata --use-svm-props --use-svnsync-props
--log-window-size= --no-checkout --quiet
--repack-flags --use-log-author --localtime
- --ignore-paths= $remote_opts
+ --ignore-paths= --include-paths= $remote_opts
local init_opts=
--template= --shared= --trunk= --tags=
diff --git a/git-svn.perl b/git-svn.perl
index 6c7bd95..5e56dc7 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -126,6 +126,7 @@ my %remote_opts = ( 'username=s' =
\$Git::SVN::Prompt::_username,
'config-dir=s' = \$Git::SVN::Ra::config_dir,
'no-auth-cache' = \$Git::SVN::Prompt::_no_auth_cache,
'ignore-paths=s' = \$Git::SVN::Fetcher::_ignore_regex,
+'include-paths=s' = \$Git::SVN::Fetcher::_include_regex,
'ignore-refs=s' = \$Git::SVN::Ra::_ignore_refs_regex );
my %fc_opts = ( 'follow-parent|follow!' = \$Git::SVN::_follow_parent,
'authors-file|A=s' = \$_authors,
@@ -470,6 +471,9 @@ sub do_git_init_db {
my $ignore_paths_regex = \$Git::SVN::Fetcher::_ignore_regex;
command_noisy('config', $pfx.ignore-paths, $$ignore_paths_regex)
if defined $$ignore_paths_regex;
+ my $include_paths_regex = \$Git::SVN::Fetcher::_include_regex;
+ command_noisy('config', $pfx.include-paths, $$include_paths_regex)
+ if defined $$include_paths_regex;
my $ignore_refs_regex = \$Git::SVN::Ra::_ignore_refs_regex;
command_noisy('config', $pfx.ignore-refs, $$ignore_refs_regex)
if defined $$ignore_refs_regex;
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 046a7a2..d25524c 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -1,6 +1,7 @@
package Git::SVN::Fetcher;
-use vars qw/@ISA $_ignore_regex $_preserve_empty_dirs $_placeholder_filename
-@deleted_gpath %added_placeholder $repo_id/;
+use vars qw/@ISA $_ignore_regex $_include_regex $_preserve_empty_dirs
+$_placeholder_filename @deleted_gpath %added_placeholder
+$repo_id/;
use strict;
use warnings;
use SVN::Delta;
@@ -33,6 +34,10 @@ sub new {
my $v = eval { command_oneline('config', '--get', $k) };
$self-{ignore_regex} = $v;
+ $k =
[ANNOUNCE] Git v1.8.3-rc1
A release candidate Git v1.8.3-rc1 is now available for testing at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 68160a9e9246a4857ccab0e68b466e0e442c1da5 git-1.8.3.rc1.tar.gz eecfe00a46a4b26b1f1a4a83f66aca2ab8b264f2 git-htmldocs-1.8.3.rc1.tar.gz 4ea043cdf8c716dddb21f60f6941cda6a12fad9e git-manpages-1.8.3.rc1.tar.gz Also the following public repositories all have a copy of the v1.8.3-rc1 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.8.3 Release Notes (draft) Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics that pushes only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. Updates since v1.8.2 Foreign interface * remote-hg and remote-bzr helpers (in contrib/) have been updated. UI, Workflows Features * The prompt string generator (in contrib/completion/) learned to show how many changes there are in total and how many have been replayed during a git rebase session. * git branch --vv learned to paint the name of the branch it integrates with in a different color (color.branch.upstream, which defaults to blue). * In a sparsely populated working tree, git checkout pathspec no longer unmarks paths that match the given pathspec that were originally ignored with --sparse (use --ignore-skip-worktree-bits option to resurrect these paths out of the index if you really want to). * git log --format specifier learned %C(auto) token that tells Git to use color when interpolating %d (decoration), %h (short commit object name), etc. for terminal output. * git bisect leaves the final outcome as a comment in its bisect log file. * git clone --reference can now refer to a gitfile textual symlink that points at the real location of the repository. * git count-objects learned --human-readable aka -H option to show various large numbers in Ki/Mi/GiB scaled as necessary. * git cherry-pick $blob and git cherry-pick $tree are nonsense, and a more readable error message e.g. can't cherry-pick a tree is given (we used to say expected exactly one commit). * The --annotate option to git send-email can be turned on (or off) by default with sendemail.annotate configuration variable (you can use --no-annotate from the command line to override it). * The --cover-letter option to git format-patch can be turned on (or off) by default with format.coverLetter configuration variable. By setting it to 'auto', you can turn it on only for a series with two or more patches. * The bash completion support (in contrib/) learned that cherry-pick takes a few more options than it already knew about. * git help learned -g option to show the list of guides just like list of commands are given with -a. * A triangular pull from one place, push to another place workflow is supported better by new
What's cooking in git.git (May 2013, #01; Fri, 3)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of the 'master' branch is tagged as v1.8.3-rc1. We seem to have a few interesting topics that are being discussed but it is unlikely I'll be picking them up in 'pu'. As we already have merged enough changes to 'master' during this cycle that can potentially cause unforseen regressions, let's not merge topics that are not regression fixes from 'next' to 'master', either, until the final release. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * hb/git-pm-tempfile (2013-04-29) 1 commit (merged to 'next' on 2013-04-29 at fecc6b0) + Git.pm: call tempfile from File::Temp as a regular function * mb/relnotes-1.8.3-typofix (2013-04-30) 1 commit - Fix grammar in the 1.8.3 release notes. * rs/pp-user-info-without-extra-allocation (2013-04-25) 3 commits (merged to 'next' on 2013-04-29 at 13eafc3) + pretty: remove intermediate strbufs from pp_user_info() + pretty: simplify output line length calculation in pp_user_info() + pretty: simplify input line length calculation in pp_user_info() * tr/remote-tighten-commandline-parsing (2013-04-24) 3 commits (merged to 'next' on 2013-04-29 at 46a1043) + remote: 'show' and 'prune' can take more than one remote + remote: check for superfluous arguments in 'git remote add' + remote: add a test for extra arguments, according to docs * tr/unpack-entry-use-after-free-fix (2013-04-30) 1 commit - unpack_entry: avoid freeing objects in base cache Fix for use-after-free regression in 1.8.3-rc0. * zk/prompt-rebase-step (2013-04-25) 1 commit (merged to 'next' on 2013-04-25 at a8264bf) + bash-prompt.sh: show where rebase is at when stopped -- [New Topics] * fc/at-head (2013-05-02) 5 commits - Add new @ shortcut for HEAD - sha1_name: refactor reinterpret() - sha1_name: compare variable with constant, not constant with variable - sha1_name: remove unnecessary braces - sha1_name: remove no-op People are too lazy to type four capital letters HEAD and want to use a single line-noise @ instead. * fc/remote-bzr (2013-04-30) 18 commits - remote-bzr: access branches only when needed - remote-bzr: delay peer branch usage - remote-bzr: iterate revisions properly - remote-bzr: improve progress reporting - remote-bzr: add option to specify branches - remote-bzr: add custom method to find branches - remote-bzr: improve author sanitazion - remote-bzr: add support for shared repo - remote-bzr: fix branch names - remote-bzr: add support for bzr repos - remote-bzr: use branch variable when appropriate - remote-bzr: fix partially pushed merge - remote-bzr: fixes for branch diverge - remote-bzr: add support to push merges - remote-bzr: always try to update the worktree - remote-bzr: fix order of locking in CustomTree - remote-bzr: delay blob fetching until the very end - remote-bzr: cleanup CustomTree * jk/lookup-object-prefer-latest (2013-05-02) 1 commit - lookup_object: prioritize recently found objects * jk/subtree-do-not-push-if-split-fails (2013-05-01) 1 commit - contrib/subtree: don't delete remote branches if split fails -- [Stalled] * mg/more-textconv (2013-04-23) 7 commits - git grep: honor textconv by default - grep: honor --textconv for the case rev:path - grep: allow to use textconv filters - t7008: demonstrate behavior of grep with textconv - cat-file: do not die on --textconv without textconv filters - show: honor --textconv for blobs - t4030: demonstrate behavior of show with textconv Rerolled. I am not sure if I like show blob and grep that use textconv by default, though. * mh/multimail (2013-04-21) 1 commit - git-multimail: a replacement for post-receive-email Waiting for comments. * jc/format-patch (2013-04-22) 2 commits - format-patch: --inline-single - format-patch: rename no_inline field A new option to send a single patch to the standard output to be appended at the bottom of a message. I personally have no need for this, but it was easy enough to cobble together. Tests, docs and stripping out more MIMEy stuff are left as exercises to interested parties. Not ready for inclusion. * jk/gitweb-utf8 (2013-04-08) 4 commits - gitweb: Fix broken blob action parameters on blob/commitdiff pages - gitweb: Don't append ';js=(0|1)' to external links - gitweb: Make feed title valid utf8 - gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain, and patch Various fixes to gitweb. Waiting for a reroll after a review. * jk/commit-info-slab (2013-04-19) 3 commits - commit-slab: introduce a macro to define a slab for new type -
[RESEND/PATCH] transport-helper: improve push messages
If there's already a remote-helper tracking ref, we can fetch the SHA-1
to report proper push messages (as opposed to always reporting
[new branch]).
The remote-helper currently can specify the old SHA-1 to avoid this
problem, but there's no point in forcing all remote-helpers to be aware
of git commit ids; they should be able to be agnostic of them.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Reviewed-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
I already sent this, but it has had the unfortunate luck of getting stuck with
a bunch of irrelevant patches in 'next'.
This is not only good enough for 'master', but probably even 'maint'.
t/t5801-remote-helpers.sh | 14 ++
transport-helper.c| 1 +
2 files changed, 15 insertions(+)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 69212cd..dbb02e2 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -186,4 +186,18 @@ test_expect_success GPG 'push signed tag with signed-tags
capability' '
compare_refs local signed-tag-2 server signed-tag-2
'
+test_expect_success 'push messages' '
+ (cd local
+ git checkout -b new_branch master
+ echo new file
+ git commit -a -m new
+ git push origin new_branch
+ git fetch origin
+ echo new file
+ git commit -a -m new
+ git push origin new_branch 2 msg
+ ! grep \[new branch\] msg
+ )
+'
+
test_done
diff --git a/transport-helper.c b/transport-helper.c
index 5f8d075..835815f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -806,6 +806,7 @@ static int push_refs_with_export(struct transport
*transport,
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
string_list_append(revlist_args, strbuf_detach(buf,
NULL));
+ hashcpy(ref-old_sha1, sha1);
}
free(private);
--
1.8.3.rc0.401.g45bba44
--
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] fast-import: only store commit objects
Felipe Contreras felipe.contre...@gmail.com writes: A safe and sane approach may be to teach these an option to tell them to omit non-commits or to emit all kinds, and make remote-bzr use that to exclude non-commits. This has nothing to do with remote-bzr, or any remote helper. These objects are not useful, not even to 'git fast-export'. If the defaults is matched to the current behaviour, nobody will get hurt Changing nothing always ensures that nobody will get hurt, but that doesn't improve anything either. I do not quite follow. Allowing existing code to depend on old behaviour, while letting new code that knows it does not need anything but commits, will get the best of both worlds. The new code will definitely improve (otherwise you won't be writing these patches), and the old code will not get hurt. Where is this doesn't improve anything coming from? -- 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] fast-import: only store commit objects
On Fri, May 3, 2013 at 6:45 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: A safe and sane approach may be to teach these an option to tell them to omit non-commits or to emit all kinds, and make remote-bzr use that to exclude non-commits. This has nothing to do with remote-bzr, or any remote helper. These objects are not useful, not even to 'git fast-export'. If the defaults is matched to the current behaviour, nobody will get hurt Changing nothing always ensures that nobody will get hurt, but that doesn't improve anything either. I do not quite follow. Allowing existing code to depend on old behaviour, while letting new code that knows it does not need anything but commits, will get the best of both worlds. The new code will definitely improve (otherwise you won't be writing these patches), and the old code will not get hurt. Where is this doesn't improve anything coming from? So let's suppose we add a new 'feature no-blob-store' to 'git fast-import', and some clients of 'git fast-import' enable it, and benefit from it. How does that benefit the rest of fast-import clients? You are worrying about clients that most likely don't exist, and don't let existing real clients benefit for free. This is like premature optimization. But whatever, let's keep writing and discarding these blobs, at least the previous patches do make such operations fast. You can drop this patch, because I'm not going to write code for clients that don't exist. And it seems clear that even if I investigate client apps of 'git fast-import' and I'm unable to find a single one that utilizes blobs, you still wouldn't apply this patch. So there's no point in investigating what are the *actual* users, if all we are every going to do is worry about hypothetical ones. My main interest is the previous patches, if anybody has any issues with the way this patch is handled, they can either work on the patch itself, or start a new thread in which I will not participate, I will rather concentrate on issues that affect *real* users, and have real fixes reachable today, and the previous patches in this series fit that bill. Cheers. -- 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 v2 0/2] Show suggested refs when ref unknown
If origin/foo exists, but foo doesn't: $ git merge foo fatal: foo - not something we can merge This patch series improves the error message. If a remote branch exists with the same name, it now says: $ git merge foo fatal: foo - not something we can merge Did you mean this? origin/foo It does this by adding a new help function, help_unknown_ref, that takes care of printing the more friendly error message, and modifies builtin/merge.c to use it. This function can easily be used by other operations involving refs that don't exist instead of providing blanket failure error messages (eg. git checkout foo). Vikrant Varma (2): help: add help_unknown_ref merge: use help_unknown_ref builtin/merge.c |3 ++- help.c | 50 ++ help.h |5 + 3 files changed, 57 insertions(+), 1 deletion(-) -- 1.7.10.4 -- 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/2] help: add help_unknown_ref
When a ref is not known, currently functions call die() with an error message.
Add helper function help_unknown_ref to take care of displaying an error
message along with a list of suggested refs the user might have meant.
Example:
$ git merge foo
merge: foo - not something we can merge
Did you mean one of these?
origin/foo
upstream/foo
Signed-off-by: Vikrant Varma vikrant.varm...@gmail.com
---
help.c | 50 ++
help.h |5 +
2 files changed, 55 insertions(+)
diff --git a/help.c b/help.c
index 02ba043..fe5fe67 100644
--- a/help.c
+++ b/help.c
@@ -7,6 +7,7 @@
#include string-list.h
#include column.h
#include version.h
+#include refs.h
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
{
@@ -404,3 +405,52 @@ int cmd_version(int argc, const char **argv, const char
*prefix)
printf(git version %s\n, git_version_string);
return 0;
}
+
+struct similar_ref_cb {
+ const char *base_ref;
+ struct string_list *similar_refs;
+};
+
+static int append_similar_ref(const char *refname, const unsigned char *sha1,
+ int flags, void *cb_data)
+{
+ struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data);
+ char *branch = strrchr(refname, '/') + 1;
+ /* A remote branch of the same name is deemed similar */
+ if (!prefixcmp(refname, refs/remotes/)
+ !strcmp(branch, cb-base_ref))
+ string_list_append(cb-similar_refs,
+ refname + strlen(refs/remotes/));
+ return 0;
+}
+
+struct string_list guess_refs(const char *ref)
+{
+ struct similar_ref_cb ref_cb;
+ struct string_list similar_refs = STRING_LIST_INIT_NODUP;
+
+ ref_cb.base_ref = ref;
+ ref_cb.similar_refs = similar_refs;
+ for_each_ref(append_similar_ref, ref_cb);
+ return similar_refs;
+}
+
+void help_unknown_ref(const char *ref, const char *cmd, const char *error)
+{
+ int i;
+ struct string_list suggested_refs = guess_refs(ref);
+
+ fprintf_ln(stderr, _(%s: %s - %s), cmd, ref, error);
+
+ if (suggested_refs.nr 0) {
+ fprintf_ln(stderr,
+ Q_(\nDid you mean this?,
+ \nDid you mean one of these?,
+ suggested_refs.nr));
+ for (i = 0; i suggested_refs.nr; i++)
+ fprintf(stderr, \t%s\n,
suggested_refs.items[i].string);
+ }
+
+ string_list_clear(suggested_refs, 0);
+ exit(1);
+}
diff --git a/help.h b/help.h
index 0ae5a12..5003423 100644
--- a/help.h
+++ b/help.h
@@ -27,4 +27,9 @@ extern void exclude_cmds(struct cmdnames *cmds, struct
cmdnames *excludes);
extern int is_in_cmdlist(struct cmdnames *cmds, const char *name);
extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds,
struct cmdnames *other_cmds);
+/*
+ * This function has been called because ref is not known.
+ * Print a list of refs that the user might have meant, and exit.
+ */
+extern void help_unknown_ref(const char *ref, const char *cmd, const char
*error);
#endif /* HELP_H */
--
1.7.10.4
--
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 2/2] merge: use help_unknown_ref
Use help.c:help_unknown_ref instead of die to provide a friendlier error
message before exiting, when one of the refs specified in a merge is unknown.
Signed-off-by: Vikrant Varma vikrant.varm...@gmail.com
---
builtin/merge.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 3e2daa3..2ebe732 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1054,7 +1054,8 @@ static struct commit_list *collect_parents(struct commit
*head_commit,
for (i = 0; i argc; i++) {
struct commit *commit = get_merge_parent(argv[i]);
if (!commit)
- die(_(%s - not something we can merge), argv[i]);
+ help_unknown_ref(argv[i], merge,
+not something we can merge);
remotes = commit_list_insert(commit, remotes)-next;
}
*remotes = NULL;
--
1.7.10.4
--
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 3/4] fast-export: don't parse all the commits
On Fri, May 3, 2013 at 4:54 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: We don't need the parsed objects at this point, merely the information that they have marks. Seems to be three times faster in my setup with lots of objects. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/fast-export.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index a5b8da8..3c5a701 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -636,7 +636,7 @@ static void import_marks(char *input_file) /* only commits */ continue; - object = parse_object(sha1); + object = lookup_unknown_object(sha1); This updates the parse_object() moved by the previous patch. At this point in the codeflow, unlike the original, we already _know_ the object must be a commit; wouldn't an equivalent of: object = (lookup_commit(sha1)-object) be more correct here? Maybe, if we want to run some extra code we don't care about. The only actual difference is that object-type will be OBJ_COMMIT, but a) this is not going to be used anywhere, and b) we can set that ourselves. In fact, my original code was: object = lookup_object(sha1); if (!object) object = create_object(sha1, OBJ_COMMIT, alloc_object_node()); But I figured there's no need for those extra lines of code. -- 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 0/2] remote-bzr: couple of fixes
Hi, A few fixes to be applied on top of the massive changes already queued. Nothing major. Felipe Contreras (2): remote-bzr: convert all unicode keys to str remote-bzr: avoid bad refs contrib/remote-helpers/git-remote-bzr | 32 1 file changed, 20 insertions(+), 12 deletions(-) -- 1.8.3.rc0.401.g45bba44 -- 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/2] remote-bzr: convert all unicode keys to str
Otherwise some versions of bazaar might barf.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 161f831..bbaaa8f 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -95,7 +95,7 @@ class Marks:
return self.marks[rev]
def to_rev(self, mark):
-return self.rev_marks[mark]
+return str(self.rev_marks[mark])
def next_mark(self):
self.last_mark += 1
@@ -621,7 +621,7 @@ def parse_commit(parser):
files[path] = f
committer, date, tz = committer
-parents = [str(mark_to_rev(p)) for p in parents]
+parents = [mark_to_rev(p) for p in parents]
revid = bzrlib.generate_ids.gen_revision_id(committer, date)
props = {}
props['branch-nick'] = branch.nick
--
1.8.3.rc0.401.g45bba44
--
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/2] remote-bzr: avoid bad refs
Turns out fast-export throws bad 'reset' commands because of a behavior
in transport-helper that is not even needed.
We should ignore them, otherwise we will threat them as branches and
fail.
This was fixed in v1.8.2, but some people use this script in older
versions of git.
Also, check if the ref was a tag, and skip it for now.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index bbaaa8f..1f48b00 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -682,23 +682,31 @@ def do_export(parser):
die('unhandled export command: %s' % line)
for ref, revid in parsed_refs.iteritems():
-name = ref[len('refs/heads/'):]
-branch = bzrlib.branch.Branch.open(branches[name])
-branch.generate_revision_history(revid, marks.get_tip(name))
+if ref.startswith('refs/heads/'):
+name = ref[len('refs/heads/'):]
+branch = bzrlib.branch.Branch.open(branches[name])
+branch.generate_revision_history(revid, marks.get_tip(name))
-if name in peers:
-peer = bzrlib.branch.Branch.open(peers[name])
+if name in peers:
+peer = bzrlib.branch.Branch.open(peers[name])
try:
peer.bzrdir.push_branch(branch, revision_id=revid)
except bzrlib.errors.DivergedBranches:
print error %s non-fast forward % ref
continue
-try:
-wt = branch.bzrdir.open_workingtree()
-wt.update()
-except bzrlib.errors.NoWorkingTree:
-pass
+try:
+wt = branch.bzrdir.open_workingtree()
+wt.update()
+except bzrlib.errors.NoWorkingTree:
+pass
+elif ref.startswith('refs/tags/'):
+# TODO: implement tag push
+print error %s pushing tags not supported % ref
+continue
+else:
+# transport-helper/fast-export bugs
+continue
print ok %s % ref
--
1.8.3.rc0.401.g45bba44
--
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/2] remote-bzr: couple of fixes
On Fri, May 3, 2013 at 7:22 PM, Felipe Contreras felipe.contre...@gmail.com wrote: A few fixes to be applied on top of the massive changes already queued. Nothing major. Felipe Contreras (2): remote-bzr: convert all unicode keys to str remote-bzr: avoid bad refs There's a problem with the second patch. Re-rolling. -- 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 v2 0/2] remote-bzr: couple of fixes
Hi, The previous version had an indentation bug (did I mention I hate python?). A few fixes to be applied on top of the massive changes already queued. Nothing major. Felipe Contreras (2): remote-bzr: convert all unicode keys to str remote-bzr: avoid bad refs contrib/remote-helpers/git-remote-bzr | 42 +-- 1 file changed, 25 insertions(+), 17 deletions(-) -- 1.8.3.rc0.401.g45bba44 -- 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/2] remote-bzr: convert all unicode keys to str
Otherwise some versions of bazaar might barf.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 161f831..bbaaa8f 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -95,7 +95,7 @@ class Marks:
return self.marks[rev]
def to_rev(self, mark):
-return self.rev_marks[mark]
+return str(self.rev_marks[mark])
def next_mark(self):
self.last_mark += 1
@@ -621,7 +621,7 @@ def parse_commit(parser):
files[path] = f
committer, date, tz = committer
-parents = [str(mark_to_rev(p)) for p in parents]
+parents = [mark_to_rev(p) for p in parents]
revid = bzrlib.generate_ids.gen_revision_id(committer, date)
props = {}
props['branch-nick'] = branch.nick
--
1.8.3.rc0.401.g45bba44
--
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 2/2] remote-bzr: avoid bad refs
Turns out fast-export throws bad 'reset' commands because of a behavior
in transport-helper that is not even needed.
We should ignore them, otherwise we will threat them as branches and
fail.
This was fixed in v1.8.2, but some people use this script in older
versions of git.
Also, check if the ref was a tag, and skip it for now.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 38 +--
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index bbaaa8f..0ef30f8 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -682,23 +682,31 @@ def do_export(parser):
die('unhandled export command: %s' % line)
for ref, revid in parsed_refs.iteritems():
-name = ref[len('refs/heads/'):]
-branch = bzrlib.branch.Branch.open(branches[name])
-branch.generate_revision_history(revid, marks.get_tip(name))
+if ref.startswith('refs/heads/'):
+name = ref[len('refs/heads/'):]
+branch = bzrlib.branch.Branch.open(branches[name])
+branch.generate_revision_history(revid, marks.get_tip(name))
-if name in peers:
-peer = bzrlib.branch.Branch.open(peers[name])
-try:
-peer.bzrdir.push_branch(branch, revision_id=revid)
-except bzrlib.errors.DivergedBranches:
-print error %s non-fast forward % ref
-continue
+if name in peers:
+peer = bzrlib.branch.Branch.open(peers[name])
+try:
+peer.bzrdir.push_branch(branch, revision_id=revid)
+except bzrlib.errors.DivergedBranches:
+print error %s non-fast forward % ref
+continue
-try:
-wt = branch.bzrdir.open_workingtree()
-wt.update()
-except bzrlib.errors.NoWorkingTree:
-pass
+try:
+wt = branch.bzrdir.open_workingtree()
+wt.update()
+except bzrlib.errors.NoWorkingTree:
+pass
+elif ref.startswith('refs/tags/'):
+# TODO: implement tag push
+print error %s pushing tags not supported % ref
+continue
+else:
+# transport-helper/fast-export bugs
+continue
print ok %s % ref
--
1.8.3.rc0.401.g45bba44
--
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 0/3] interactive git clean
2013/5/3 Eric Sunshine sunsh...@sunshineco.com: WARNING: The following items will be removed permanently. Press y WARNING: to start cleaning, and press n to abort the cleaning. WARNING: You can also enter the edit mode, and select items WARNING: to be excluded from the cleaning. The user intended for files to be removed when invoking git-clean, therefore WARNING that git-clean will do what was requested explicitly seems overkill. Along the same lines, the user asked explicitly for an interactive session (via --interactive), hence the above paragraph is effectively redundant since it does little more than tell the user (in a lengthy fashion) what he already knows (that the session is interactive). The short prompt printed after the listed files says the same thing (more succinctly), thus this warning paragraph is essentially superfluous. I will try to make the header short, and not too scary. In this confirmation dialog, the user has three choices: * Yes: Start to do cleaning. * No: Nothing will be deleted. * Edit (default for the first time): Enter the edit mode. What about the user who desires more traditional rm -i behavior in which he is prompted for each file? Should that be supported with a Prompt [each] option in the above menu? I'd like to have a try. Maybe I can borrow code/interface from git-add--interactive.perl to support both (batch exclude and confirm one by one). For example: *** Would remove the following item(s) *** files to be removed... files to be removed... files to be removed... files to be removed... files to be removed... files to be removed... *** Commands *** 1. [y]es, clean2. [n]o, quit3. batch [e]xclude 4.[c]onfirm one by one What now e input ignore patterns * ![a-c]* files to be removed... files to be removed... files to be removed... files to be removed... Input ignore patterns ENTER *** Would remove the following item(s) *** files to be removed... files to be removed... files to be removed... files to be removed... *** Commands *** 1. [y]es, clean2. [n]o, quit3. batch [e]xclude 4.[c]onfirm one by one What now y Removing ... Removing ... More generally, is this sort of modal edit mode desirable and convenient? Can the edit operation be combined with the top-level prompt? For example: % git clean -i file1 file2 file3 file4 file5 file6 Remove ([y]es, [n]o, [p]rompt, exclusion-list)? file[4-6] file1 file2 file3 Remove ([y]es, [n]o, [p]rompt, exclusion-list)? p file1 (y/n/q/!)? y file2 (y/n/q/!)? n file3 (y/n/q/!)? y What If there is a file named 'y', and the user want to exclude it, and press 'y' as a pattern. -- Jiang Xin -- 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 v3 4/4] clone: open a shortcut for connectivity check
On Fri, May 3, 2013 at 11:15 PM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: In order to make sure the cloned repository is good, we run rev-list --objects --not --all $new_refs on the repository. This is expensive on large repositories. This patch attempts to mitigate the impact in this special case. In the good clone case, we only have one pack. If On large repositories is the focus, we need to take into account the fact that pack.packSizeLimit can split and store the incoming packstream to multiple packs, so only have one pack is misleading. I only had a quick look. But I don't think index-pack respects packSizeLimit. pack-objects does but only when --stdout is not used, which is not the case for pack transfer. I think you can still do the same trick even when we split the pack as index-pack will keep track of the objects it saw in the same incoming pack stream (but I am writing this from memory without looking at the original code you are touching, so please double check). Yeah. As long we have only one incoming stream, we can still do the same verification. index-pack + new checks is still faster than the current index-pack + rev-list, which is the whole point of this patch. If any of the Does the same check apply if we end up on the unpack-objects codepath? No. unpack-objects does not do this and check_everything_connected should invoke rev-list like before. -- 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
[PATCH v2] clone: allow cloning local paths with colons in them
Usually foo:bar is interpreted as an ssh url. This patch allows to
clone from such paths by putting at least one slash before the colon
(i.e. /path/to/foo:bar or just ./foo:bar).
file://foo:bar should also work, but local optimizations are off in
that case, which may be unwanted. While at there, warn the users about
--local being ignored in this case.
Reported-by: William Giokas 1007...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Changes from v1: replace strchr with strchrnul.
Documentation/urls.txt | 6 ++
builtin/clone.c| 2 ++
connect.c | 7 +--
t/t5601-clone.sh | 5 +
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 3ca122f..476e338 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -23,6 +23,12 @@ An alternative scp-like syntax may also be used with the ssh
protocol:
- {startsb}user@{endsb}host.xz:path/to/repo.git/
+This syntax is only recognized if there are no slashes before the
+first colon. This helps differentiate a local path that contains a
+colon. For example the local path `foo:bar` could be specified as an
+absolute path or `./foo:bar` to avoid being misinterpreted as an ssh
+url.
+
The ssh and git protocols additionally support ~username expansion:
-
ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/
diff --git a/builtin/clone.c b/builtin/clone.c
index 58fee98..e13da4d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -783,6 +783,8 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
is_local = option_local != 0 path !is_bundle;
if (is_local option_depth)
warning(_(--depth is ignored in local clones; use file://
instead.));
+ if (option_local 0 !is_local)
+ warning(_(--local is ignored));
if (argc == 2)
dir = xstrdup(argv[1]);
diff --git a/connect.c b/connect.c
index f57efd0..a0783d4 100644
--- a/connect.c
+++ b/connect.c
@@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
path = strchr(end, c);
if (path !has_dos_drive_prefix(end)) {
if (c == ':') {
- protocol = PROTO_SSH;
- *path++ = '\0';
+ if (path strchrnul(host, '/')) {
+ protocol = PROTO_SSH;
+ *path++ = '\0';
+ } else /* '/' in the host part, assume local path */
+ path = end;
}
} else
path = end;
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..0629149 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '
test_cmp fetch.expected fetch.actual
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
+ cp -R src foo:bar
+ git clone ./foo:bar foobar
+'
+
test_done
--
1.8.2.83.gc99314b
--
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] remove the impression of unexpectedness when access is denied
Hi,
Heiko Voigt wrote:
--- a/connect.c
+++ b/connect.c
@@ -49,6 +49,16 @@ static void add_extra_have(struct extra_have_objects
*extra, unsigned char *sha1
extra-nr++;
}
+static void die_initial_contact(int got_at_least_one_head)
+{
+ if (got_at_least_one_head)
+ die(The remote end hung up upon initial contact);
+ else
+ die(Could not read from remote repository.\n\n
+ Please make sure you have the correct access rights\n
+ and the repository exists.);
+}
[...]
I ran into this message for the first time today.
$ git fetch --all
Fetching origin
remote: Counting objects: 368, done.
[...]
Fetching gitk
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
error: Could not fetch gitk
Fetching debian
Fetching pape
[...]
The gitk remote refers to git://git.kernel.org/pub/scm/gitk/gitk.
Using ls-remote to contact it produces the same result. The message
is correct: the repository does not exist.
Impressions:
* Looking at Could not read, it is not clear what could not read
and why. GIT_TRACE_PACKET tells me the interaction was
me git-upload-pack /pub/scm/gitk/gitk\0host=git.kernel.org\0
them (hangup)
Would it make sense for the server to send an ERR packet to give
a more helpful diagnosis?
* The spacing and capitalization is odd and makes it not flow well
with the rest of the output. I suspect it would be easier to read
with the error separated from hints:
Fetching gitk
fatal: the remote server sent an empty response
hint: does the repository exist?
hint: do you have the correct access rights?
error: Could not fetch gitk
Fetching debian
If a server is misconfigured and just decides to send an empty
response for no good reason, the output would still be true.
* The error message is the same whether the server returned no
response or an incomplete pkt-line. Maybe in the latter case it
should print the hung up unexpectedly thing.
Thoughts?
Jonathan
--
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: Another use of @?
On Sat, May 4, 2013 at 5:09 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
The A/@ could make sense, but I'm wondering whether we're taking the
direction of implementing some kind of Brainfuck dialect in Git revision
specifiers. I'm not sure we want to add more special characters here and
there with subtly different meanings (@ = HEAD, @{1} = HEAD@{1}, A/@ =
A/$(git symbolic-ref --short HEAD)).
Another subtle overloading of @ that might be desirable (althought
might be achievable another way). git log -g is equal to git log
-g HEAD but there is no easy way (that I know of) to do git log -g
$(git symbolic-ref HEAD). @ could fill the inconvenient spot here,
I think. Alias is no good because I won't be able to add extra
options.
--
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
[PATCH] get_sha1: improve ambiguity warning regarding SHA-1 and ref names
When we get 40 hex digits, we immediately assume it's an SHA-1. Warn
about ambiguity if there's also refs/heads/$sha1 (or similar) on system.
When we successfully resolve a ref like 1234abc and 1234abc
happens to be valid abbreviated SHA-1 on system, warn also.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Thu, May 2, 2013 at 1:43 AM, Jonathan Nieder jrnie...@gmail.com wrote:
Nguyễn Thái Ngọc Duy wrote:
git rev-parse 1234 will
resolve refs/heads/1234 if exists even if there is an unambiguous
SHA-1 starting with 1234. However if it's full SHA-1, the SHA-1 takes
precedence and refs with the same name are ignored.
That's an important feature for safety. When a script has created an
object or learned about it some other way, as long as it doesn't
abbreviate its name it can be sure that git commands will not
misunderstand it.
So I think this is a bad change. Maybe check-ref-format should
reject 40-hexdigit refnames?
We can't, at least not in a simple way, because there are 40-hex
digit refs in refs/replace. I think warning only is a simpler
approach to this minor issue.
sha1_name.c | 12 ++--
t/t1512-rev-parse-disambiguation.sh | 18 ++
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..04a9fbe 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -435,12 +435,18 @@ static int get_sha1_1(const char *name, int len, unsigned
char *sha1, unsigned l
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
{
static const char *warn_msg = refname '%.*s' is ambiguous.;
+ unsigned char tmp_sha1[20];
char *real_ref = NULL;
int refs_found = 0;
int at, reflog_len;
- if (len == 40 !get_sha1_hex(str, sha1))
+ if (len == 40 !get_sha1_hex(str, sha1)) {
+ refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
+ if (refs_found 0 warn_ambiguous_refs)
+ warning(warn_msg, len, str);
+ free(real_ref);
return 0;
+ }
/* basic@{time or number or -number} format to query ref-log */
reflog_len = at = 0;
@@ -481,7 +487,9 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (!refs_found)
return -1;
- if (warn_ambiguous_refs refs_found 1)
+ if (warn_ambiguous_refs
+ (refs_found 1 ||
+!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
warning(warn_msg, len, str);
if (reflog_len) {
diff --git a/t/t1512-rev-parse-disambiguation.sh
b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..db22808 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -261,4 +261,22 @@ test_expect_success 'rev-parse --disambiguate' '
test $(sed -e s/^\(.\).*/\1/ actual | sort -u) = 0
'
+test_expect_success 'ambiguous 40-hex ref' '
+ TREE=$(git mktree /dev/null)
+ REF=`git rev-parse HEAD`
+ VAL=$(git commit-tree $TREE /dev/null)
+ git update-ref refs/heads/$REF $VAL
+ test `git rev-parse $REF 2err` = $REF
+ grep refname.*${REF}.*ambiguous err
+'
+
+test_expect_success 'ambiguous short sha1 ref' '
+ TREE=$(git mktree /dev/null)
+ REF=`git rev-parse --short HEAD`
+ VAL=$(git commit-tree $TREE /dev/null)
+ git update-ref refs/heads/$REF $VAL
+ test `git rev-parse $REF 2err` = $VAL
+ grep refname.*${REF}.*ambiguous err
+'
+
test_done
--
1.8.2.83.gc99314b
--
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: Another use of @?
On Sat, May 4, 2013 at 10:26 AM, Duy Nguyen pclo...@gmail.com wrote:
On Sat, May 4, 2013 at 5:09 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
The A/@ could make sense, but I'm wondering whether we're taking the
direction of implementing some kind of Brainfuck dialect in Git revision
specifiers. I'm not sure we want to add more special characters here and
there with subtly different meanings (@ = HEAD, @{1} = HEAD@{1}, A/@ =
A/$(git symbolic-ref --short HEAD)).
Another subtle overloading of @ that might be desirable (althought
might be achievable another way). git log -g is equal to git log
-g HEAD but there is no easy way (that I know of) to do git log -g
$(git symbolic-ref HEAD). @ could fill the inconvenient spot here,
I think. Alias is no good because I won't be able to add extra
options.
I wouldn't mind typing ref@{link} that does git symbolic-ref
ref, though. Because ref is optional, git log -g @{link} is not
bad. link is probably not a good name for this.
--
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
