Re: A couple of rebase --autosquash proposals
On 09/12/13 19:51, Johannes Sixt wrote: Am 12/9/2013 3:23, schrieb Brett Randall: * fixup! or squash! on it's own would default to fixing-up the previous commit (or result of previous step of rebase if that was a squash/fixup). Why would you want that? To fixup the previous commit, just use 'git commit --amend'. What am I missing? In the past I've used this kind of approach when doing merging/porting work with 3rd party code (or just large integrations). The first (and eventually final) commit introduces the new code. The subsequent fixups address build issues which are either errors in the 3rd party code (which I will want to submit bug reports for later and carry in my tree as real commits) or errors in my merging (which I want to squash into the merge commit). When faced with a screen full of compilation errors I'm not sure which of these 2 categories are applicable at the time so I tend to have lots of little fixups that I need to juggle around with git rebase once I've got the code compiling and passing some tests. All that being said I think allowing multiple fixup!\n stack up on each other might be a bit dangerous. There are cases where fixup!-fixup!-real might be useful but those would be hard to distinguish those from cases where someone absent mindedly forgot to put something after fixup!. -- 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: A couple of rebase --autosquash proposals
This aims to support code-review workflows of teams that prefer rebase over merge, when committing a new peer-reviewed feature. * Developer starts with commit OM, commits A. * During testing, the developer may make further changes, either through --amend or new commits, but either way, all work is rebased to a single new commit for review, OM - A' . * A' is pushed as a new branch to origin for team review. The review system facilitates the review of the change, and review comments are made. * The developer responds to the review comments by making changes in commits B and C, and pushes OM - A' - B - C. Reviewers can understand the feedback that has been addressed in the changes with through the commit-log in B and C. * Code passes review. Because the team prefers rebased commits, A'..C is rebased onto the current OM (which may now be OM+10) and committed. If the commit-log entries for B and C allow simultaneous fixup!/squash! syntax together with and free-text log-text, they can serve both purposes: 1) they communicate that the change is a feedback-generated fix (rather than a new feature), and describe which parts of the feedback each commit addresses, and 2) they pre-empt and support the eventual rebase-before-origin-push, through --autosquash annotation. Brett On 9 December 2013 20:26, Chris Packham judge.pack...@gmail.com wrote: On 09/12/13 19:51, Johannes Sixt wrote: Am 12/9/2013 3:23, schrieb Brett Randall: * fixup! or squash! on it's own would default to fixing-up the previous commit (or result of previous step of rebase if that was a squash/fixup). Why would you want that? To fixup the previous commit, just use 'git commit --amend'. What am I missing? In the past I've used this kind of approach when doing merging/porting work with 3rd party code (or just large integrations). The first (and eventually final) commit introduces the new code. The subsequent fixups address build issues which are either errors in the 3rd party code (which I will want to submit bug reports for later and carry in my tree as real commits) or errors in my merging (which I want to squash into the merge commit). When faced with a screen full of compilation errors I'm not sure which of these 2 categories are applicable at the time so I tend to have lots of little fixups that I need to juggle around with git rebase once I've got the code compiling and passing some tests. All that being said I think allowing multiple fixup!\n stack up on each other might be a bit dangerous. There are cases where fixup!-fixup!-real might be useful but those would be hard to distinguish those from cases where someone absent mindedly forgot to put something after fixup!. -- 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 00/24] Index-v5
Thomas Gummerer t.gumme...@gmail.com writes: Hi, previous rounds (without api) are at $gmane/202752, $gmane/202923, $gmane/203088 and $gmane/203517, the previous rounds with api were at $gmane/229732, $gmane/230210 and $gmane/232488. Thanks to Duy for reviewing the the last round and Junio, Ramsay and Eric for additional comments. Since the last round I've added a POC for partial writing, resulting in the following performance improvements for update-index: Test1063432 HEAD 0003.2: v[23]: update-index 0.60(0.38+0.20) 0.76(0.36+0.17) +26.7% 0003.3: v[23]: grep nonexistent -- subdir 0.28(0.17+0.11) 0.28(0.18+0.09) +0.0% 0003.4: v[23]: ls-files -- subdir 0.26(0.15+0.10) 0.24(0.14+0.09) -7.7% 0003.7: v[23] update-index 0.59(0.36+0.22) 0.58(0.36+0.20) -1.7% 0003.9: v4: update-index0.46(0.28+0.17) 0.45(0.30+0.11) -2.2% 0003.10: v4: grep nonexistent -- subdir 0.26(0.14+0.11) 0.21(0.14+0.07) -19.2% 0003.11: v4: ls-files -- subdir 0.24(0.14+0.10) 0.20(0.12+0.08) -16.7% 0003.14: v4 update-index0.49(0.31+0.18) 0.65(0.34+0.17) +32.7% 0003.16: v5: update-index 0.53(0.30+0.22) 0.50(0.28+0.20) -5.7% 0003.17: v5: ls-files 0.27(0.15+0.12) 0.27(0.17+0.10) +0.0% 0003.18: v5: grep nonexistent -- subdir 0.02(0.01+0.01) 0.03(0.01+0.01) +50.0% 0003.19: v5: ls-files -- subdir 0.02(0.00+0.02) 0.02(0.01+0.01) +0.0% 0003.22: v5 update-index0.53(0.29+0.23) 0.02(0.01+0.01) -96.2% Given this, I don't think a complete change of the in-core format for the cache-entries is necessary to take full advantage of the new index file format. Instead some changes to the current in-core format would work well with the new on-disk format. The current in-memory format fits the internal needs of git fairly well, so I don't think changing it to fit a better index file format would make a lot of sense, given that we can take advantage of the new format with the existing in-memory format. Any more opinions on this series? I've applied the changes suggested by Duy, Antoine and Eric locally, but I wouldn't want to spam the list with the whole series without a chance of this being applied. How do you want me to proceed? This series doesn't use kb/fast-hashmap yet, but that should be fairly simple to change if the series is deemed a good change. The performance tests for update-index test require tg/perf-lib-test-perf-cleanup. Other changes, made following the review comments are: documentation: add documentation of the index-v5 file format - Update documentation that directory flags are now 32-bits. That makes aligned access simpler - offset_to_offset is no longer included in the checksum for files. It's unnecessary. read-cache: read index-v5 - Add fix for reading with different level pathspecs given - Use init_directory_entry to initialize all fields in a new directory entry - use memset to simplify the create_new_conflict function - Add comments to explain -5 when reading directories and files - Add comments for the more complex functions - Add name flex_array to the end of ondisk_directory_entry for simplified reading - Add name flex_array to the end of ondisk_cache_entry for simplified reading - Move conflict reading functions to next patch - mark functions as static when they are read-cache: read resolve-undo data - Add comments for the more complex function - Read conflicts + resolve undo data as extension read-cache: read cache-tree in index-v5 - Add comments for the more complex function - Instead of sorting the directory entries, sort the cache-tree directly. This also required changing the algorithms with which the cache entries are extracted from the directory tree. read-cache: write index-v5 - Free pointers allocated by super_directory - Rewrite condition as suggested by Duy - Don't check for CE_REMOVE'd entries in the writing code, they are already checked in the compile_directory_data code - Remove overly complicated directory size calculation since flags are now 32-bits read-cache: write resolve-undo data for index-v5 - Free pointers allocated by super_directory - Write conflicts + resolve undo data as extension introduce GIT_INDEX_VERSION environment variable - Add documentation for GIT_INDEX_VERSION test-lib: allow setting the index format version Removed commits: - read-cache: don't check uid, gid, ino - read-cache: use fixed width integer types (independently in pu) - read-cache: clear version in discard_index() Typos fixed as suggested by Eric Sunshine Thomas Gummerer (22): read-cache:
mv/rm submodules
Hi list, I was just playing with the new features of 'git mv' in 1.8.5 and realized that both rm and mv don't change the .git/config file. Is that on purpuse? Also after mv you need to run 'submodule update' and I think this should be documented somewhere. Thanks.. -- George 'papanikge' Papanikolaou g3orge@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
Setting file timestamps to commit time (git-checkout)
Me and some colleagues work on gcc in lots of different branches. For each branch there is a separate build directory for each branch, e.g. build-a, build-b and build-c. Let's assume that all branches are identical at the moment. If a file in branch a is changed that triggers a complete rebuild of gcc (e.g. target.opt), rebuilding in build-a takes about an hour. Now, when I switch to one of the other branches, said file is not identical anymore and stamped with the _current_ time during checkout. Although branch b and c have not changed at all, they will now be rebuilt completely because the timestamp on that files has changed. I.e. a chance on one branch forces a rebuild on n other branches, which can take many hours. I think this situation could be improved with an option to git-checkout with the following logic: $ git checkout new branch FOR EACH file in working directory of new branch IF file is identical to the version in the old branch THEN leave the file untouched ELSE IF commit timestamp of the HEAD of the new branch is in the future THEN checkout the new version of file and stamp it with the current time ELSE (commit timestamp is current or in the past) THEN checkout the new version of file and stamp it with the commit timestamp of the current HEAD of new branch Any comments? Is there already a way to do this? (Please do not cc me on replies, I'm subscribed to the list.) Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany -- 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] diff: don't read index when --no-index is given
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). In the usual case this gives us some
performance drawbacks, but it's especially annoying if there is a broken
index file.
Avoid calling the unnecessary gitmodules_config() when the --no-index
option is given. Also add a test to guard against similar breakages in the
future.
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
builtin/diff.c | 13 +++--
t/t4053-diff-no-index.sh | 6 ++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..47c0833 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -257,7 +257,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
- int nongit;
+ int nongit, no_index = 0;
int result = 0;
/*
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+ for (i = 1; i argc; i++) {
+ if (!strcmp(argv[i], --))
+ break;
+ if (!strcmp(argv[i], --no-index)) {
+ no_index = 1;
+ break;
+ }
+ }
prefix = setup_git_directory_gently(nongit);
- gitmodules_config();
+ if (!no_index)
+ gitmodules_config();
git_config(git_diff_ui_config, NULL);
init_revisions(rev, prefix);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 979e983..a24ae4d 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path
outside repo' '
)
'
+test_expect_success 'git diff --no-index with broken index' '
+ cd repo
+ echo broken .git/index
+ test_expect_code 0 git diff --no-index a ../non/git/a
+'
+
test_done
--
1.8.4.2
--
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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 08.12.2013 11:20, schrieb Thomas Rast:
Karsten Blees karsten.bl...@gmail.com writes:
Am 07.12.2013 23:23, schrieb Thomas Rast:
Karsten Blees karsten.bl...@gmail.com writes:
Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but needs '__attribute__((__packed__))' to work. Reduces e.g.
You'd have to guard __attribute__((__packed__)) with some compiler
detection in git-compat-util.h though.
Isn't that already handled? __attribute__ is already widely used
(e.g. for printf formats), and platforms that don't support it define
it as empty (e.g. MSVC). Or do you mean I should account for
compiler-specific variants (#pragma pack...)?
True, __attribute__ expands to nothing on unknown compilers, but what
does the compiler do when it sees an unknown attribute? If some of them
choke, you need a separate macro.
I'm a bit confused myself though, many attributes have special macros in
git-compat-util.h but others we just use in the code.
So what do you propose? I basically see three options:
1.) Trial and error
GCC supports __packed__ as of 2.3 (1992), so any other compilers that copied
the __attribute__ feature probably won't complain.
2.) Accept the wasted memory
Moving the hash code from the hash table (as in hash.[ch]) to the entry is
already a big improvement, as it no longer multiplies hash code + wasted bytes
with the load factor. 64-bit software uses more memory in general, so we could
just live with it (and only fix the documentation in api-hashmap.txt).
3.) Inject individual fields via macro
Instead of injecting a struct hashmap_entry, which implies alignment to
sizeof(struct hashmap_entry), we could inject the individual fields, e.g.
#define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int __hash;
struct name_entry {
HASHMAP_ENTRY_HEADER
int namelen;
char *name;
};
What do you think?
Karsten
--
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] diff: don't read index when --no-index is given
Thomas Gummerer wrote:
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). In the usual case this gives us some
performance drawbacks,
Makes sense.
but it's especially annoying if there is a broken
index file.
Is this really a normal case? It makes sense that as a side-effect it
is easier to use git diff --no-index as a general-purpose tool while
investigating a broken repo, but I would have thought that quickly
learning a repo is broken is a good thing in any case.
A little more information about the context where this came up would
be helpful, I guess.
[...]
--- a/builtin/diff.c
+++ b/builtin/diff.c
[...]
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+ for (i = 1; i argc; i++) {
+ if (!strcmp(argv[i], --))
+ break;
+ if (!strcmp(argv[i], --no-index)) {
+ no_index = 1;
+ break;
+ }
setup_revisions() uses the same logic that doesn't handle options that
take arguments in their unstuck form (e.g., --word-diff-regex --),
so this is probably not a regression, though I haven't checked. :)
[...]
prefix = setup_git_directory_gently(nongit);
- gitmodules_config();
+ if (!no_index)
+ gitmodules_config();
Perhaps we can improve performance and behavior by skipping the
setup_git_directory_gently() call, too?
That would help with the repairing-broken-repository case by
working even if .git/config or .git/HEAD is broken, and I think
it is more intuitive that the repository-local configuration is
ignored by git diff --no-index. It would also help with
performance by avoiding some filesystem access.
[...]
+test_expect_success 'git diff --no-index with broken index' '
+ cd repo
+ echo broken .git/index
+ test_expect_code 0 git diff --no-index a ../non/git/a
Clever. I wouldn't use test_expect_code 0, since that's
already implied by including the git diff --no-index call
in the chain.
Thanks and hope that helps,
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: [PATCH] diff: don't read index when --no-index is given
Am 09.12.2013 16:16, schrieb Jonathan Nieder: Thomas Gummerer wrote: git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). In the usual case this gives us some performance drawbacks, Makes sense. Hmm, but this will disable the submodule specific ignore configuration options defined in the .gitmodules file, no? (E.g. when diffing two directories containing submodules) but it's especially annoying if there is a broken index file. Is this really a normal case? It makes sense that as a side-effect it is easier to use git diff --no-index as a general-purpose tool while investigating a broken repo, but I would have thought that quickly learning a repo is broken is a good thing in any case. But I agree that dying with index file corrupt is a bit strange when calling diff with --no-index. Wouldn't adding a gently option (which could then warn instead of dying) to gitmodules_config() be a better solution here? -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees karsten.bl...@gmail.com writes: Am 07.12.2013 00:52, schrieb Junio C Hamano: * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit - gitignore.txt: clarify recursive nature of excluded directories Originally merged to 'next' on 2013-11-13 Kicked back to 'pu' to replace with a newer reroll ($gmane/237814 looked OK but there seems to have some loose ends in the discussion). I'm unaware of any loose ends, could you clarify? Btw. $gmane/237814 seems to be a different topic, the version in next (and now in pu) was $gmane/237429. Ah, my mistake. I thought I still had $gmane/237416 and needed to wait for the discussion to conclude; in fact, a later one in that thread $gmane/237429 is what came out of that discussion, so this is good to go, perhaps with an amend with Acked-by: peff. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees karsten.bl...@gmail.com writes:
* kb/fast-hashmap (2013-11-18) 14 commits
(merged to 'next' on 2013-12-06 at f90be3d)
Damn, a day too late :-) I found these two glitches today...is a
fixup patch OK or should I do a reroll (or separate patch on top)?
A separate patch on top would be the most appropriate. People have
been looking at the change since mid November, and nobody noticed
the problem; having a separate fix on top is a good way to document
what the specific gotcha that can be easily missed is.
I think the patch you attached describes the issue well, possibly
with a retitle (perhaps hashmap.h: make sure map entries are
tightly packed, or something.)
Thanks.
--- 8 ---
Subject: [PATCH] fixup! add a hashtable implementation that supports O(1)
removal
Use 'unsigned int' for hash-codes everywhere.
Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but needs '__attribute__((__packed__))' to work. Reduces e.g.
struct name_entry {
struct hashmap_entry ent;
int namelen;
char *name;
};
from 32 to 24 bytes.
Signed-off-by: Karsten Blees bl...@dcon.de
---
hashmap.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hashmap.h b/hashmap.h
index f5b3b61..b64567b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len);
/* data structures */
-struct hashmap_entry {
+struct __attribute__((__packed__)) hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
@@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int
free_entries);
/* hashmap_entry functions */
-static inline void hashmap_entry_init(void *entry, int hash)
+static inline void hashmap_entry_init(void *entry, unsigned int hash)
{
struct hashmap_entry *e = entry;
e-hash = hash;
--
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] contrib/git-credential-gnome-keyring.c: small stylistic cleanups
John Szakmeister j...@szakmeister.net writes:
Signed-off-by: John Szakmeister j...@szakmeister.net
---
The gnome-keyring credential backend had a number of coding style
violations. I believe this fixes all of them.
.../gnome-keyring/git-credential-gnome-keyring.c | 55
++
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 635c96b..1613404 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -95,9 +95,9 @@ static const char*
gnome_keyring_result_to_message(GnomeKeyringResult result)
static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer
user_data)
{
- gpointer *data = (gpointer*) user_data;
- int *done = (int*) data[0];
- GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
+ gpointer *data = (gpointer *) user_data;
+ int *done = (int *) data[0];
+ GnomeKeyringResult *r = (GnomeKeyringResult *) data[1];
I thought we cast without SP after the (typename), i.e.
gpointer *data = (gpointer *)user_data;
It could be argued that a cast that turns a void * to a pointer to
another type can go, as Felipe noted, but I think that is better
done in a separate patch, perhaps as a follow-up to this small
stylistic clean-ups.
I said it could be argued above, because I am on the fence on that
change. If this were not using a type gpointer, whose point is to
hide what the actual implementation of that type is, but a plain
vanilla void *, then I would not have any doubt. But it feels
wrong to look behind that deliberate gpointer abstraction and take
advantage of the knowledge that it happens to be implemented as
void * (and if we do not start from that knowledge, losing the
cast is a wrong change).
So...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] rev-parse and --
Jeff King p...@peff.net writes: Is it better for rev-parse to be more careful, and to behave more like the rest of git? Or is better to be historically compatible? One thing to note is that git rev-parse HEAD is slightly broken there already. Because git rev-parse $some_branch may do very different things than what the caller expects if $some_branch does not exist (but there is a file with the same name). So maybe we are doing a favor by calling out the problem; if they want a rev, they should be using --verify (or --). I tend to agree with the reasoning in the last sentence. Let's cook it for a while and see what happens. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: don't read index when --no-index is given
Jens Lehmann wrote: Am 09.12.2013 16:16, schrieb Jonathan Nieder: Thomas Gummerer wrote: git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). In the usual case this gives us some performance drawbacks, Makes sense. Hmm, but this will disable the submodule specific ignore configuration options defined in the .gitmodules file, no? (E.g. when diffing two directories containing submodules) Yes. That seems like a good behavior. git diff --no-index was invented as just a fancy version of 'diff -uR, without any awareness of the current git repository. That means that at least in principle, .gitmodules and .gitignore should not affect it. [...] Wouldn't adding a gently option (which could then warn instead of dying) to gitmodules_config() be a better solution here? I don't think so. Thanks and hope that helps, 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: [PATCH v2 0/3] rev-parse and --
Junio C Hamano wrote: So maybe we are doing a favor by calling out the problem; if they want a rev, they should be using --verify (or --). I tend to agree with the reasoning in the last sentence. Let's cook it for a while and see what happens. Isn't this essentially breaking a contract that would have been relied on by any script that used git rev-parse HEAD~3..HEAD? Worse, it's breaking that contract in a way that no one would notice until they are asked to manipulate a worktree with a file named 'HEAD~3..HEAD' --- in other words, the breakage it introduces is painfully subtle. I agree that git rev-parse HEAD is better written as git rev-parse --verify HEAD and hence not so much worth worrying about, but I don't find it easy to believe that people should have anticipated this change and added a trailing -- to more complex rev-parse expressions. So to be clear, I think unless it is protected by a new option, this is a bad idea. Hope that helps, 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: [PATCH] diff: don't read index when --no-index is given
Jonathan Nieder jrnie...@gmail.com writes:
Thomas Gummerer wrote:
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). In the usual case this gives us some
performance drawbacks,
Makes sense.
but it's especially annoying if there is a broken
index file.
Is this really a normal case? It makes sense that as a side-effect it
is easier to use git diff --no-index as a general-purpose tool while
investigating a broken repo, but I would have thought that quickly
learning a repo is broken is a good thing in any case.
A little more information about the context where this came up would
be helpful, I guess.
It came up while I was working on index-v5, where I had to investigate
quite a few repositories where the index was broken, especially when I
was changing the index format slightly. For example I would take one
version, use test-dump-cache-tree to dump the cache tree to a file,
change the format slightly, use test-dump-cache-tree again, and check
the difference with git diff --no-index.
This might not be a very common use case, but maybe the patch might help
someone else too. (In addition to the performance improvements)
I'm not sure how much diff --no-index is used normally, but when the
index is broken that would be detected relatively soon anyway. I'm not
so worried about one more command working when it's broken.
[...]
--- a/builtin/diff.c
+++ b/builtin/diff.c
[...]
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+for (i = 1; i argc; i++) {
+if (!strcmp(argv[i], --))
+break;
+if (!strcmp(argv[i], --no-index)) {
+no_index = 1;
+break;
+}
setup_revisions() uses the same logic that doesn't handle options that
take arguments in their unstuck form (e.g., --word-diff-regex --),
so this is probably not a regression, though I haven't checked. :)
The same logic is used in diff_no_index(), so I think it should be fine.
[...]
prefix = setup_git_directory_gently(nongit);
-gitmodules_config();
+if (!no_index)
+gitmodules_config();
Perhaps we can improve performance and behavior by skipping the
setup_git_directory_gently() call, too?
That would help with the repairing-broken-repository case by
working even if .git/config or .git/HEAD is broken, and I think
it is more intuitive that the repository-local configuration is
ignored by git diff --no-index. It would also help with
performance by avoiding some filesystem access.
Yes, I think that would make sense, thanks. I tested it, and it didn't
change the performance, but it's still a good change for the broken
repository case. Will change in the re-roll and add a test for that.
[...]
+test_expect_success 'git diff --no-index with broken index' '
+cd repo
+echo broken .git/index
+test_expect_code 0 git diff --no-index a ../non/git/a
Clever. I wouldn't use test_expect_code 0, since that's
already implied by including the git diff --no-index call
in the chain.
Thanks, will change. Thanks a lot for your review. Will send a re-roll
soon.
Thanks and hope that helps,
Jonathan
--
Thomas
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] rev-parse and --
Jonathan Nieder wrote: Isn't this essentially breaking a contract To clarify, if there were some strong motivation --- e.g. if this were resolving some security problem --- then I would be all for breaking compatibility in this way. What's confusing to me is that I just don't see the motivation. Why wouldn't someone that wants this functionality be able to pass a flag for it? Is the idea that without a flag it's easier to teach or something? Hoping that clarifies, 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: [PATCH v2] diff: Add diff.orderfile configuration variable
Samuel Bronson naes...@gmail.com writes:
If somebody has diff.orderfile configuration that points at a custom
ordering, and wants to send out a patch (or show a diff) with the
standard order, how would the overriding command line look like?
Would it be git diff -O/dev/null?
It looks like that works ... and so do files that don't exist. What
do you think should happen with -O file-that-does-not-exist, and how
do you suppose it should be tested?
I think the original code is too loose on diagnosing errors.
Perhaps something like this is needed.
diff --git a/diffcore-order.c b/diffcore-order.c
index 23e9385..dd103e3 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -20,8 +20,11 @@ static void prepare_order(const char *orderfile)
return;
fd = open(orderfile, O_RDONLY);
- if (fd 0)
+ if (fd 0) {
+ if (errno != ENOENT || errno != ENOTDIR)
+ die(_(orderfile '%s' does not exist.));
return;
+ }
if (fstat(fd, st)) {
close(fd);
return;
After having fixed this, will /dev/null still work everywhere, or will
we want a new diff flag to unset the option? (I see that git diff
/dev/null some-file works fine with msysgit, which doesn't seem to
actually be linked with MSYS, but I don't know *why* it works, and I
don't know what other non-POSIXoid ports exist.)
I *think* it should be OK to use -O/dev/null for that purpose, but
the primary thing I was hinting at with the rhetoric question was
that it probably needs to be documented there.
Also, I'm starting to wonder if I shouldn't split this into two patches:
1. diff: Add tests for -O flag
2. diff: Add diff.orderfile configuration variable
(If so, I would obviously want to rewrite the above test to avoid the
configuration option.)
Surely, and thanks.
EOF
cat order_file_2 -\EOF
I'd kind of prefer to keep a blank line between one EOF and the next
cat, if that's okay with you.
Alright. Making it easier to spot the grouping, it would make it
easier to read.
Quoting the EOF like the above will help the readers by signaling
them that they do not have to wonder if there is some substitution
going on in the here text.
Perhaps, but probably only after they've scrutinized their shell
manuals to figure out what the - and the \ are for. (I had to check
two: dash(1) wasn't clear enough for me about the quoting ...)
Yes and no. The no primarily comes from that nobody will stay to be
novice forever.
--
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] i18n: move the trailing space out of translatable strings
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
I've got this with Vietnamese translation
$ git status
HEAD được tách rời từorigin/master
One does not need to understand Vietnamese to see that a space is
missing before origin/master...
Not really. Only if one guesses (or knows) that Vietnamese is among
the languages that use SP to separate words. There are languages
that do not use inter-word spaces.
Because those languages are in the minority, it would be easier to
unconditionally add SP after translatable string regardless of the
l10n target languages, but I am afraid that it is going in a wrong
direction in the longer term.
clean_print_color(CLEAN_COLOR_PROMPT);
- printf(_(Input ignore patterns ));
+ printf(%s , _(Input ignore patterns));
clean_print_color(CLEAN_COLOR_RESET);
As this space after the prompt may be different from inter-word
space after all, this one may probably be on the borderline.
@@ -754,7 +754,8 @@ static int ask_each_cmd(void)
/* Ctrl-D should stop removing files */
if (!eof) {
qname = quote_path_relative(item-string, NULL, buf);
- printf(_(remove %s? ), qname);
+ printf(_(remove %s?), qname);
+ putchar(' ');
As is this change.
But it is an example that can be used to illustrate my earlier
point. The l10n target language may want to have _no_ space between
words, i.e. to turn the above into:
printf(%sを削除しますか?, qname);
putchar(' ');
i.e. no space between the object of the verb (the path being
removed) and the verb (to remove), while still wanting to have SP
between the prompt and its answer like everybody else. And having
SP after remove in the gettext key _is_ a good thing, as l10n
people can choose not to have any SP between the verb and its
object.
diff --git a/builtin/clone.c b/builtin/clone.c
index 874e0fd..d48ccee 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
if (check_connectivity) {
if (transport-progress)
- fprintf(stderr, _(Checking connectivity... ));
+ fprintf(stderr, _(Checking connectivity...));
if (check_everything_connected_with_transport(iterate_ref_map,
0, rm,
transport))
die(_(remote did not send all necessary objects));
if (transport-progress)
- fprintf(stderr, _(done.\n));
+ fprintf(stderr, %s, _(done.\n));
But I think that this changes enforces the inter-word SP policy that
could go against convention by specific l10n target languages.
--
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] t5541: Improve push test
The old log-line looked like this: + 9d498b0...8598732 master - master (forced update) And the new one like this: 9d498b0..8598732 master - master - Loosen the grep pattern by not demanding (forced update) - Improve the grep pattern and check the new SHA id Signed-off-by: Torsten Bögershausen tbo...@web.de --- The following revealed a weakness in t5541: commit f9e3c6bebb89de12f2dfdaa1899cb22e9ef32542 Author: Felipe Contreras felipe.contre...@gmail.com Date: Tue Nov 12 14:56:57 2013 -0600 transport-helper: check for 'forced update' message So don't look for forced update, but check for the SHA. (I want to fix a missing as well, that is for the next commit) diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index 470ac54..1468a07 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -168,7 +168,8 @@ test_expect_success 'push fails for non-fast-forward refs unmatched by remote he test_must_fail git push -v origin +master master:retsam output 21' test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper: remote output' ' - grep ^ + [a-f0-9]*\.\.\.[a-f0-9]* *master - master (forced update)$ output + newsha=$(git log --oneline -n1 | sed -e s/^\([0-9a-f]*\).*/\1/) + grep \.\.$newsha *master - master output grep ^ ! \[rejected\] *master - retsam (non-fast-forward)$ output ' -- 1.8.5 -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees wrote: GCC supports __packed__ as of 2.3 (1992), so any other compilers that copied the __attribute__ feature probably won't complain. Alas, it looks like HP C doesn't support __packed__ (not that I care much about HP C): http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes Maybe a macro expanding to __attribute__((aligned(1))) on the fields would work? The same macro could expand to __declspec(align(1)) in the MSVC build. Thanks, 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: [RFC/PATCH] rebase: use reflog to find common base with upstream
John Keeping j...@keeping.me.uk writes:
Last time this came up [1], there was some discussion about moving the
added block of code to affect upstreams given on the command line as
well as when the upstream is discovered from the config. Having tried
that, it has some more fallout on the test suite than I like; this
pattern ends up dropping the tip commit of side because it's in the
reflog of master:
# start on master
git branch side
git reset --hard HEAD^
git checkout side
git rebase master
We shouldn't do anything funky using the reflog when an explicit
commit object name was given like in the last step above, I think.
Automation to help human end-users is good, but at some level there
must be a mechanism to reliably reproduce the same result given the
same precondition for those who implement such automation, and I do
not think it is a good idea to force scripts to say
git rebase --do-not-look-at-reflog master
in order to do so.
I wonder if it would be better to add a --fork-point argument to
git-rebase and default it to true when no upstream is given on the
command line.
I am not sure what you exactly mean by when no upstream is given,
though. Do you mean
git rebase no other arguments
which we interpret as rebase the current branch on @{u}, and it
should behave as if the command was run like so:
git rebase --fork-point @{u}
If that is what you suggest, I certainly can buy that. Those who
want to disable the automation can explicitly say
git rebase @{u}
and rebase the current exactly on top of the named commit (e.g. the
current value of refs/remotes/origin/master or whatever remote-tracking
branch you forked 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: A couple of rebase --autosquash proposals
Johannes Sixt j.s...@viscovery.net writes: Am 12/9/2013 3:23, schrieb Brett Randall: * fixup! or squash! on it's own would default to fixing-up the previous commit (or result of previous step of rebase if that was a squash/fixup). Why would you want that? To fixup the previous commit, just use 'git commit --amend'. What am I missing? When you are not absolutely sure if the amend is a good thing to do. Then work work work git commit --fixup HEAD work work work git commit --fixup HEAD^ work work work git commit --fixup HEAD^^ ... git rebase --autosquash -i ... may become a good way to polish a single commit. -- 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] diff: don't read index when --no-index is given
Thomas Gummerer t.gumme...@gmail.com writes:
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). In the usual case this gives us some
performance drawbacks, but it's especially annoying if there is a broken
index file.
Avoid calling the unnecessary gitmodules_config() when the --no-index
option is given. Also add a test to guard against similar breakages in the
future.
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
builtin/diff.c | 13 +++--
t/t4053-diff-no-index.sh | 6 ++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..47c0833 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -257,7 +257,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
- int nongit;
+ int nongit, no_index = 0;
int result = 0;
/*
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+ for (i = 1; i argc; i++) {
+ if (!strcmp(argv[i], --))
+ break;
+ if (!strcmp(argv[i], --no-index)) {
+ no_index = 1;
+ break;
+ }
+ }
This seems to duplicate only half the logic at the beginning of
diff_no_index(), right? E.g., running git diff /var/tmp/[12]
inside a working tree that is controlled by a Git repository when
/var/tmp/ is outside, we do want to behave as if the command line
were git diff --no-index /var/tmp/[12], but this half duplication
makes these two behave differently, no?
I think the issue you are trying to address is worth tackling, but I
wonder if a bit of preparatory refactoring is necessary to avoid the
partial duplication.
prefix = setup_git_directory_gently(nongit);
- gitmodules_config();
+ if (!no_index)
+ gitmodules_config();
git_config(git_diff_ui_config, NULL);
init_revisions(rev, prefix);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 979e983..a24ae4d 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path
outside repo' '
)
'
+test_expect_success 'git diff --no-index with broken index' '
+ cd repo
+ echo broken .git/index
+ test_expect_code 0 git diff --no-index a ../non/git/a
+'
+
test_done
--
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: Setting file timestamps to commit time (git-checkout)
Dominik Vogt v...@linux.vnet.ibm.com writes: Me and some colleagues work on gcc in lots of different branches. For each branch there is a separate build directory for each branch, e.g. build-a, build-b and build-c. Let's assume that all branches are identical at the moment. If a file in branch a is changed that triggers a complete rebuild of gcc (e.g. target.opt), rebuilding in build-a takes about an hour. Now, when I switch to one of the other branches, said file is not identical anymore and stamped with the _current_ time during checkout. Although branch b and c have not changed at all, they will now be rebuilt completely because the timestamp on that files has changed. I am not quite sure I follow your set-up. Do you have three working trees connected to a repository (via contrib/workdir/git-new-workdir perhaps), each having a checkout of its own branch? And in one working directory that has build-a checked out, a new commit touches one file, target.opt, to make a new commit: Before: ---o---o---X ^ refs/heads/build-a refs/heads/build-b refs/heads/build-c After: v refs/heads/build-a ---o---o---X---Y ^ refs/heads/build-b refs/heads/build-c Because you said that branch b and c hasn't changed at all, I do not see how your build-b and/or build-c directories become dirty. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] rebase: use reflog to find common base with upstream
On Mon, Dec 09, 2013 at 12:11:50PM -0800, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
Last time this came up [1], there was some discussion about moving the
added block of code to affect upstreams given on the command line as
well as when the upstream is discovered from the config. Having tried
that, it has some more fallout on the test suite than I like; this
pattern ends up dropping the tip commit of side because it's in the
reflog of master:
# start on master
git branch side
git reset --hard HEAD^
git checkout side
git rebase master
We shouldn't do anything funky using the reflog when an explicit
commit object name was given like in the last step above, I think.
Automation to help human end-users is good, but at some level there
must be a mechanism to reliably reproduce the same result given the
same precondition for those who implement such automation, and I do
not think it is a good idea to force scripts to say
git rebase --do-not-look-at-reflog master
in order to do so.
I wonder if it would be better to add a --fork-point argument to
git-rebase and default it to true when no upstream is given on the
command line.
I am not sure what you exactly mean by when no upstream is given,
though. Do you mean
git rebase no other arguments
which we interpret as rebase the current branch on @{u}, and it
should behave as if the command was run like so:
git rebase --fork-point @{u}
If that is what you suggest, I certainly can buy that. Those who
want to disable the automation can explicitly say
git rebase @{u}
and rebase the current exactly on top of the named commit (e.g. the
current value of refs/remotes/origin/master or whatever remote-tracking
branch you forked from).
Yes, that's what I meant; the first non-option argument to git rebase
is called upstream in the manpage (and throughout the code). So if
no other arguments means no non-option arguments then that's
exactly what I meant.
--
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] diff: don't read index when --no-index is given
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). This results in worse performance when
the index is not actually needed. This patch avoids calling
gitmodules_config() when the --no-index option is given. The times for
executing git diff --no-index in the WebKit repository are improved as
follows:
Test HEAD~3HEAD
--
4001.1: diff --no-index 0.24(0.15+0.09) 0.01(0.00+0.00) -95.8%
An additional improvement of this patch is that git diff --no-index no
longer breaks when the index file is corrupt, which makes it possible to
use it for investigating the broken repository.
To improve the possible usage as investigation tool for broken
repositories, setup_git_directory_gently() is also not called when the
--no-index option is given.
Also add a test to guard against future breakages, and a performance
test to show the improvements.
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
Thanks to Jonathan and Jens for comments on the previous round.
Changes:
- Don't all setup_git_directory_gently when --no-index is given
- Add performance test
- Commit message improvements
builtin/diff.c| 16 +---
t/perf/p4001-diff-no-index.sh | 17 +
t/t4053-diff-no-index.sh | 6 ++
3 files changed, 36 insertions(+), 3 deletions(-)
create mode 100755 t/perf/p4001-diff-no-index.sh
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..5f09a0b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -257,7 +257,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
- int nongit;
+ int nongit, no_index = 0;
int result = 0;
/*
@@ -282,9 +282,19 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+ for (i = 1; i argc; i++) {
+ if (!strcmp(argv[i], --))
+ break;
+ if (!strcmp(argv[i], --no-index)) {
+ no_index = 1;
+ break;
+ }
+ }
- prefix = setup_git_directory_gently(nongit);
- gitmodules_config();
+ if (!no_index) {
+ prefix = setup_git_directory_gently(nongit);
+ gitmodules_config();
+ }
git_config(git_diff_ui_config, NULL);
init_revisions(rev, prefix);
diff --git a/t/perf/p4001-diff-no-index.sh b/t/perf/p4001-diff-no-index.sh
new file mode 100755
index 000..81c7aa0
--- /dev/null
+++ b/t/perf/p4001-diff-no-index.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+test_description=Test diff --no-index performance
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+file1=$(git ls-files | tail -n 2 | head -1)
+file2=$(git ls-files | tail -n 1 | head -1)
+
+test_perf diff --no-index
+ git diff --no-index $file1 $file2 /dev/null
+
+
+test_done
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 979e983..d3dbf6b 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path
outside repo' '
)
'
+test_expect_success 'git diff --no-index with broken index' '
+ cd repo
+ echo broken .git/index
+ git diff --no-index a ../non/git/a
+'
+
test_done
--
1.8.5.4.g8639e57
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] rev-parse and --
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: So maybe we are doing a favor by calling out the problem; if they want a rev, they should be using --verify (or --). I tend to agree with the reasoning in the last sentence. Let's cook it for a while and see what happens. Isn't this essentially breaking a contract that would have been relied on by any script that used git rev-parse HEAD~3..HEAD? Worse, it's breaking that contract in a way that no one would notice until they are asked to manipulate a worktree with a file named 'HEAD~3..HEAD' --- in other words, the breakage it introduces is painfully subtle. I agree that git rev-parse HEAD is better written as git rev-parse --verify HEAD and hence not so much worth worrying about, I do not share the with --verify is better hence no problem reasoning. My not so much worth worrying about comes solely from a hunch that nobody has HEAD~3..HEAD in their working directory, and if somebody has one, then they must be using --verify (or a clarifying --), because their git log and whatever they use git rev-parse HEAD~3..HEAD for would behave very differently otherwise. So it is not merely --verify is better---in a situation where the backward incompatibility matters, I doubt the existing behaves sensibly in the first place. But if we cook it for a while, I suspect that we will find more and more breakages of expectations in the existing scripts in and out of the tree; in general, I hate _any_ change, and I do not mind dropping 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
Re: Setting file timestamps to commit time (git-checkout)
Hi, Dominik Vogt wrote: Now, when I switch to one of the other branches, said file is not identical anymore and stamped with the _current_ time during checkout. Although branch b and c have not changed at all, they will now be rebuilt completely because the timestamp on that files has changed. I.e. a chance on one branch forces a rebuild on n other branches, which can take many hours. I think this situation could be improved with an option to git-checkout with the following logic: $ git checkout new branch FOR EACH file in working directory of new branch IF file is identical to the version in the old branch THEN leave the file untouched ELSE IF commit timestamp of the HEAD of the new branch is in the future THEN checkout the new version of file and stamp it with the current time ELSE (commit timestamp is current or in the past) THEN checkout the new version of file and stamp it with the commit timestamp of the current HEAD of new branch Wouldn't that break make? When you switch to an old branch, changed files would then a timestamp *before* the corresponding build targets, causing the stale (wrong function signatures, etc) build results from the newer branch to be reused and breaking the build. I suspect the simplest way to accomplish what you're looking for would be to keep separate worktrees for each branch you regularly build. It's possible to do that using entirely independent clones, clones sharing some objects (using git clone --shared from some master copy), or even multiple worktrees for the same clone (using the git-new-workdir script from contrib/workdir/). See [1] and [2] for more hints. [...] (Please do not cc me on replies, I'm subscribed to the list.) The convention on this list is to always reply-to-all, but I'm happy to make an exception. :) Hope that helps, Jonathan [1] https://git.wiki.kernel.org/index.php/Git_FAQ#Why_isn.27t_Git_preserving_modification_time_on_files.3F [2] https://git.wiki.kernel.org/index.php/ExampleScripts#Setting_the_timestamps_of_the_files_to_the_commit_timestamp_of_the_commit_which_last_touched_them -- 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] diff: don't read index when --no-index is given
On 2013-12-09 21.40, Thomas Gummerer wrote: +test_expect_success 'git diff --no-index with broken index' ' + cd repo + echo broken .git/index + git diff --no-index a ../non/git/a ^^ I'm confused: Does this work with the trailing ? (and when we use cd repo it could be good to use a sub-shell (even when this is the last test case) -- 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
[RFC/WIP PATCH] implement reading of submodule .gitmodules configuration into cache
This submodule configuration cache allows us to lazily read .gitmodules
configurations by commit into a runtime cache which can then be used to
easily lookup values from it. Currently only the values for path or name
are stored but it can be extended for any value needed.
This cache can be used for all purposes which need knowledge about
submodule configurations.
It is expected that .gitmodules files do not change often between
commits. Thats why we lookup the .gitmodules sha1 from a commit and then
either lookup an already parsed configuration or parse and cache an
unknown one for each sha1. The cache is lazily build on demand for each
requested commit.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
On Tue, Dec 03, 2013 at 07:33:01PM +0100, Heiko Voigt wrote:
On Mon, Dec 02, 2013 at 03:55:36PM -0800, Nick Townsend wrote:
It seems to me that the question that you are trying to solve is
more complex than the problem I faced in git-archive, where we have a
single commit of the top-level repository that we are chasing.
Perhaps we should split the work into two pieces:
a. Identifying the complete submodule configuration for a single commit, and
b. the complexity of behaviour when fetching and cloning recursively (which
of course requires a.)
You are right the latter (b) is a separate topic. So how about I extract the
submodule config parsing part from the mentioned patch and you can then
use that patch as a basis for your work? As far as I understand you only
need to parse the .gitmodules file for one commit and then lookup the
submodule names from paths right? That would simplify matters and we can
postpone the caching of multiple commits for the time when I continue on b.
Ok and here is a patch that you can use as basis for your work. I looked
into it and found that its actually easier to use the cache. Storing
everything in a hashmap seems like overkill for your application but it
is prepared for my usecase which actually needs to parse configurations
for multiple commits.
Since this has not been discussed yet some details of my implementation
might change.
The test I implemented is only for demonstration of usage and my quick
manual testing. If we agree on going ahead with my patch I will extend
that to a proper test. Or if anyone has an idea where we can plug that
in to make a useful user interface we can also implement a test based on
that.
Cheers Heiko
.gitignore| 1 +
Makefile | 2 +
submodule-config-cache.c | 96
submodule-config-cache.h | 33 +++
submodule.c | 125 ++
submodule.h | 4 ++
test-submodule-config-cache.c | 45 +++
7 files changed, 306 insertions(+)
create mode 100644 submodule-config-cache.c
create mode 100644 submodule-config-cache.h
create mode 100644 test-submodule-config-cache.c
diff --git a/.gitignore b/.gitignore
index 66199ed..6c91e98 100644
--- a/.gitignore
+++ b/.gitignore
@@ -200,6 +200,7 @@
/test-sha1
/test-sigchain
/test-string-list
+/test-submodule-config-cache
/test-subprocess
/test-svn-fe
/test-urlmatch-normalization
diff --git a/Makefile b/Makefile
index af847f8..e3d869b 100644
--- a/Makefile
+++ b/Makefile
@@ -572,6 +572,7 @@ TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
TEST_PROGRAMS_NEED_X += test-string-list
+TEST_PROGRAMS_NEED_X += test-submodule-config-cache
TEST_PROGRAMS_NEED_X += test-subprocess
TEST_PROGRAMS_NEED_X += test-svn-fe
TEST_PROGRAMS_NEED_X += test-urlmatch-normalization
@@ -872,6 +873,7 @@ LIB_OBJS += strbuf.o
LIB_OBJS += streaming.o
LIB_OBJS += string-list.o
LIB_OBJS += submodule.o
+LIB_OBJS += submodule-config-cache.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
diff --git a/submodule-config-cache.c b/submodule-config-cache.c
new file mode 100644
index 000..7253fad
--- /dev/null
+++ b/submodule-config-cache.c
@@ -0,0 +1,96 @@
+#include cache.h
+#include submodule-config-cache.h
+#include strbuf.h
+#include hash.h
+
+void submodule_config_cache_init(struct submodule_config_cache *cache)
+{
+ init_hash(cache-for_name);
+ init_hash(cache-for_path);
+}
+
+static int free_one_submodule_config(void *ptr, void *data)
+{
+ struct submodule_config *entry = ptr;
+
+ strbuf_release(entry-path);
+ strbuf_release(entry-name);
+ free(entry);
+
+ return 0;
+}
+
+void submodule_config_cache_free(struct submodule_config_cache *cache)
+{
+ /* NOTE: its important to iterate over the name hash here
+* since paths might have multiple entries */
+ for_each_hash(cache-for_name, free_one_submodule_config, NULL);
+ free_hash(cache-for_path);
+ free_hash(cache-for_name);
+}
+
+static unsigned int hash_sha1_string(const unsigned char *sha1, const char
*string)
+{
Re: [PATCH v2 0/3] rev-parse and --
Junio C Hamano wrote: I do not share the with --verify is better hence no problem reasoning. My not so much worth worrying about comes solely from a hunch that nobody has HEAD~3..HEAD in their working directory, That's what makes it a problem. This change makes it very easy to make a general-purpose script that breaks in an edge case that the script's author is not likely to run into. Then as soon as someone adds a file with such a name to the test data in their repo, their favorite general-purpose repo munger just breaks. If we wanted to forbid git from tracking files named HEAD~3..HEAD altogether, that would be a different story. and if somebody has one, then they must be using --verify (or a clarifying --), because their git log and whatever they use git rev-parse HEAD~3..HEAD for would behave very differently otherwise. Isn't protecting against this kind of thing the reason we ask authors of general-purpose scripts to use simple, do what I say and not what I mean plumbing commands? Another relevant detail is that using rev-parse with -- is more painful than without, since it includes the -- in its output. Without this change, it seems much more likely to me that someone would do git rev-parse commits | while read commit do ... done than git rev-parse commits -- | while read commit do if test $commit = -- then continue fi ... done So it is not merely --verify is better---in a situation where the backward incompatibility matters, I doubt the existing behaves sensibly in the first place. What in the former of the above two loops is broken? But if we cook it for a while, I suspect that we will find more and more breakages of expectations in the existing scripts in and out of the tree; Alas, probably no, because nobody has HEAD~3..HEAD in their working directory. That's exactly the problem --- it creates an edge case that nobody is likely to test until the once-in-a-few-years moment when they need it. 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: [PATCH v2] diff: don't read index when --no-index is given
On Mon, Dec 9, 2013 at 3:40 PM, Thomas Gummerer t.gumme...@gmail.com wrote: git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). This results in worse performance when the index is not actually needed. This patch avoids calling gitmodules_config() when the --no-index option is given. The times for executing git diff --no-index in the WebKit repository are improved as follows: Test HEAD~3HEAD -- 4001.1: diff --no-index 0.24(0.15+0.09) 0.01(0.00+0.00) -95.8% An additional improvement of this patch is that git diff --no-index no longer breaks when the index file is corrupt, which makes it possible to use it for investigating the broken repository. To improve the possible usage as investigation tool for broken repositories, setup_git_directory_gently() is also not called when the --no-index option is given. Also add a test to guard against future breakages, and a performance test to show the improvements. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 979e983..d3dbf6b 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path outside repo' ' ) ' +test_expect_success 'git diff --no-index with broken index' ' + cd repo + echo broken .git/index + git diff --no-index a ../non/git/a +' Stray on the last line of the test. Also, don't you want to do the 'cd' and subsequent commands inside a subshell so that tests added after this one won't have to worry about cd'ing back to the top-level? + test_done -- 1.8.5.4.g8639e57 -- 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 v6 06/10] fast-export: add new --refspec option
Felipe Contreras felipe.contre...@gmail.com writes:
Junio C Hamano wrote:
Junio C Hamano gits...@pobox.com writes:
Felipe Contreras felipe.contre...@gmail.com writes:
But it does not have to stay that way. In order to move things
forward in that direction, this new configuration has to be
distinguishable from the traditional refspec, as it embodies a
different concept.
Is it a different concept?
% git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
% git fetch origin master
What do you call this thing? --^
The answer to that question is the value of the 'remote.*.fetch'
configuration variable.
The refspec mechanism and syntax used in fetch and push were
invented and designed to express two things [*1*]:
1) what is the set of objects that are necessary and sufficient to
complete some histories need to be transferred; what are the
tips of these histories that are being completed?
2) after the object transfer, some refs on the side that receive
the objects are optionally to be updated;
2-a) which refs are they?
2-b) what objects are they updated to point at?
A refspec consists of one or more elements, each of which has
right and left hand side separated by a colon, i.e. RHS:LHS, and
1) is determined by the RHS
2-a) is determined by the LHS
2-b) is determined by the correspondence between RHS-to-LHS.
A wildcarded refs/heads/*:refs/remotes/origin/* dynamically
expands to what the side that sends objects have, e.g. if fetching
from a repository with 'master' and 'next' branches, it becomes
refs/heads/master:refs/remotes/origin/master
refs/heads/next:refs/remotes/origin/next
So with
$ refspec='refs/heads/master:refs/remotes/origin/master
refs/heads/next:refs/remotes/origin/next'
$ git fetch origin $refspec
we transfer objects to allow us to have complete histories leading
to 'master' and 'next' from the origin repository. And we update
our refs/remotes/origin/{next,master} refs.
Traditionally, when there is _no_ refspec on the command line, the
value of 'remote.*.fetch' configuration variable is used as the
fallback default, and that usage is still true. When used in that
way, the value of the variable _is taken as_ a refspec.
However, we can no longer say that the variable _is_ a refspec with
the modern Git, and here is why.
git fetch origin master used to ignore the 'remote.*.fetch'
configuration variable completely, but since f2690487 (which is a
fairly recent invention), the variable participates in the fetch
process in a different way [*2*]. With this in the config:
[remote origin]
fetch = refs/heads/master:refs/remotes/origin/master
fetch = refs/heads/next:refs/remotes/origin/next
the command with 'master' refspec on the command line transfers the
objects required to complete the history only for the 'master', and
not 'next':
$ git fetch origin master
In this usage, 'master' on the command line is the only thing that
determines what histories are completed (because it does not say
'next', the objects necessary to complete its history are not
transferred unless they are needed to complete 'master'). The value
of the 'remote.*.fetch' configuration does not participate in the
determination of the history being transferred at all. It is not
used as a refspec.
But unlike Git of the last year, we do map this 'master' using the
'remote.*.fetch' configuration variable, in order to decide 2)
above. We find that the given remote ref, 'master', has an element
that corresopnds to it in the 'remote.*.fetch' configuration, and
that element tells us to update refs/remotes/origin/master to point
at the object they call 'master', effectively turning the above
command line into this form (using refspec that has only one
element, refs/heads/master:refs/remotes/origin/master):
$ git fetch origin refs/heads/master:refs/remotes/origin/master
There is no refs/heads/next:refs/remotes/origin/next here, because
the 'fetch' configuration is not used as a refspec, but as something
else.
My understanding of the added option parameter to git fast-export
is that it is not about specifying the history being transferred,
but is about mapping the name of the destination. For example, does
object between 'master' and 'next' participate in the datastream
produced with this?
fast-export \
--refspec=refs/heads/master:refs/remotes/origin/master \
--refspec=refs/heads/next:refs/remotes/origin/next \
master
If this parameter were a refspec, as we have discussed already in
previous rounds [*3*], we should be able to give it on the command line,
like any normal refspec, instead of repeating the same thing
(i.e. up to what commit should the history be transported) as in:
fast-export --refspec=refs/heads/master:refs/remotes/origin/master
master
but just
fast-export
Re: [PATCH v8 1/6] transport-helper: mismerge fix
Will re-queue these 6 for now. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] rev-parse and --
Jonathan Nieder jrnie...@gmail.com writes: But if we cook it for a while, I suspect that we will find more and more breakages of expectations in the existing scripts in and out of the tree; Alas, probably no, because nobody has HEAD~3..HEAD in their working directory. That's exactly the problem --- it creates an edge case that nobody is likely to test... I didn't mean to say that running and encoutering the issue is the only way to find more breakges of expectations, by the way. I am still on the fence, but leaning towards not merging it too hastily. -- 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 v6 06/10] fast-export: add new --refspec option
Junio C Hamano gits...@pobox.com writes: A refspec consists of one or more elements, each of which has right and left hand side separated by a colon, i.e. RHS:LHS, and 1) is determined by the RHS 2-a) is determined by the LHS 2-b) is determined by the correspondence between RHS-to-LHS. Heh, I got my right and left the other way around ;-) -- 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] diff: don't read index when --no-index is given
[Sorry for not seeing this before sending out v2.]
Junio C Hamano gits...@pobox.com writes:
Thomas Gummerer t.gumme...@gmail.com writes:
git diff --no-index ... currently reads the index, during setup, when
calling gitmodules_config(). In the usual case this gives us some
performance drawbacks, but it's especially annoying if there is a broken
index file.
Avoid calling the unnecessary gitmodules_config() when the --no-index
option is given. Also add a test to guard against similar breakages in the
future.
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
builtin/diff.c | 13 +++--
t/t4053-diff-no-index.sh | 6 ++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..47c0833 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -257,7 +257,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
-int nongit;
+int nongit, no_index = 0;
int result = 0;
/*
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
*
* Other cases are errors.
*/
+for (i = 1; i argc; i++) {
+if (!strcmp(argv[i], --))
+break;
+if (!strcmp(argv[i], --no-index)) {
+no_index = 1;
+break;
+}
+}
This seems to duplicate only half the logic at the beginning of
diff_no_index(), right? E.g., running git diff /var/tmp/[12]
inside a working tree that is controlled by a Git repository when
/var/tmp/ is outside, we do want to behave as if the command line
were git diff --no-index /var/tmp/[12], but this half duplication
makes these two behave differently, no?
Yes you're right, I missed that. git diff /var/tmp/[12] inside a
working tree with a broken index has the same problems, which should be
fixed too. I'll try to fix that and send a new patch afterwards.
I think the issue you are trying to address is worth tackling, but I
wonder if a bit of preparatory refactoring is necessary to avoid the
partial duplication.
prefix = setup_git_directory_gently(nongit);
-gitmodules_config();
+if (!no_index)
+gitmodules_config();
git_config(git_diff_ui_config, NULL);
init_revisions(rev, prefix);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 979e983..a24ae4d 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path
outside repo' '
)
'
+test_expect_success 'git diff --no-index with broken index' '
+cd repo
+echo broken .git/index
+test_expect_code 0 git diff --no-index a ../non/git/a
+'
+
test_done
--
Thomas
--
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] diff: don't read index when --no-index is given
Eric Sunshine sunsh...@sunshineco.com writes: On Mon, Dec 9, 2013 at 3:40 PM, Thomas Gummerer t.gumme...@gmail.com wrote: git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). This results in worse performance when the index is not actually needed. This patch avoids calling gitmodules_config() when the --no-index option is given. The times for executing git diff --no-index in the WebKit repository are improved as follows: Test HEAD~3HEAD -- 4001.1: diff --no-index 0.24(0.15+0.09) 0.01(0.00+0.00) -95.8% An additional improvement of this patch is that git diff --no-index no longer breaks when the index file is corrupt, which makes it possible to use it for investigating the broken repository. To improve the possible usage as investigation tool for broken repositories, setup_git_directory_gently() is also not called when the --no-index option is given. Also add a test to guard against future breakages, and a performance test to show the improvements. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 979e983..d3dbf6b 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -29,4 +29,10 @@ test_expect_success 'git diff --no-index relative path outside repo' ' ) ' +test_expect_success 'git diff --no-index with broken index' ' + cd repo + echo broken .git/index + git diff --no-index a ../non/git/a +' Stray on the last line of the test. Also, don't you want to do the 'cd' and subsequent commands inside a subshell so that tests added after this one won't have to worry about cd'ing back to the top-level? Thanks both to you and Torsten for catching both issues, I'll fix them in a re-roll. + test_done -- 1.8.5.4.g8639e57 -- Thomas -- 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] remote: fix status with branch...rebase=preserve
Felipe Contreras felipe.contre...@gmail.com writes:
Commit 66713ef (pull: allow pull to preserve merges when rebasing)
didn't include an update so 'git remote status' parses
branch.name.rebase=preserve
correctly, let's do that.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/remote.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 4e14891..5e4ab66 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -309,8 +309,13 @@ static int config_read_branches(const char *key, const
char *value, void *cb)
space = strchr(value, ' ');
}
string_list_append(info-merge, xstrdup(value));
- } else
- info-rebase = git_config_bool(orig_key, value);
+ } else {
+ int v = git_config_maybe_bool(orig_key, value);
+ if (v = 0)
+ info-rebase = v;
+ else if (!strcmp(value, preserve))
+ info-rebase = 1;
+ }
Looks correct. Do we want to add a test?
}
return 0;
}
--
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] remote: fix status with branch...rebase=preserve
Thanks, will queue. -- 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/7] abspath: trivial style fix
Felipe Contreras felipe.contre...@gmail.com writes:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
abspath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/abspath.c b/abspath.c
index e390994..8b3385a 100644
--- a/abspath.c
+++ b/abspath.c
@@ -143,7 +143,7 @@ static const char *real_path_internal(const char *path,
int die_on_error)
error_out:
free(last_elem);
if (*cwd chdir(cwd))
- die_errno (Could not change back to '%s', cwd);
+ die_errno(Could not change back to '%s', cwd);
Will queue; thanks.
By the way, there are quite a few hits from
git grep -e '\(die\|error\|warning\) (' -- \*.c
even if we ignore borrowed code that we do not want to touch up with
this kind of change. Some may have overlapping changes in flight
that makes it better to leave them as they are for now, but some are
in reasonably quiescent areas.
--
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 5/7] fetch: add missing documentation
Felipe Contreras felipe.contre...@gmail.com writes: There's no mention of the 'origin' default, or the fact that the upstream tracking branch remote is used. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/git-fetch.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index e08a028..a7b245d 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -37,6 +37,9 @@ or from several repositories at once if group is given and there is a remotes.group entry in the configuration file. (See linkgit:git-config[1]). +When no remote is specified, by default the `origin` remote will be used, +unless there's an upstream branch configured for the current branch. + It is better than saying nothing, but the reader will be left wondering what happens when upstream branch is configured with the above text. When no remote is specified, by default the `origin` remote will be used, unless there's an upstream branch configured for the current branch, in which case the `fetch` goes to the remote the specified upstream remote-tracking branch is taken from. or something, perhaps? OPTIONS --- include::fetch-options.txt[] -- 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/3] rev-parse: be more careful with munging arguments
On Fri, Dec 6, 2013 at 5:07 PM, Jeff King p...@peff.net wrote: When rev-parse looks at whether an argument like foo..bar or foobar^@ is a difference or parent-shorthand, it internally munges the arguments so that it can pass the individual rev arguments to get_sha1. However, we do not consistently un-munge the result. For cases where we do not match (e.g., doesnotexist..HEAD), we wouuld then want to try to treat the argument as a s/wouuld/would/ filename. try_difference gets this right, and always unmunges in this case. However, try_parent_shorthand never unmunges, leading to incorrect error messages, or even incorrect results: $ git rev-parse foobar^@ foobar fatal: ambiguous argument 'foobar': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' $ foobar $ git rev-parse foobar^@ foobar For cases where we do match, neither function unmunges. This does not currently matter, since we are done with the argument. However, a future patch will do further processing, and this prepares for it. In addition, it's simply a confusing interface for some cases to modify the const argument, and others not to. Signed-off-by: Jeff King p...@peff.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: Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all
On Thu, Dec 05, 2013 at 09:51:31PM +0100, Jens Lehmann wrote: Am 05.12.2013 00:19, schrieb Heiko Voigt: On Wed, Dec 04, 2013 at 02:32:46PM -0800, Junio C Hamano wrote: This series tries to achieve the following goals for the submodule.name.ignore=all configuration or the --ignore-submodules=all command line switch. Thanks for the summary. * Make git status never ignore submodule changes that got somehow in the index. Currently when ignore=all is specified they are and thus secretly committed. Basically always show exactly what will be committed. Yes, what's in the index should always be shown as such even when the user chose to ignore the work tree differences of the submodule. * Make add ignore submodules that have the ignore=all configuration when not explicitly naming a certain submodule (i.e. using git add .). That way ignore=all submodules are not added to the index by default. That can be overridden by using the -f switch so it behaves the same as with untracked files specified in one of the ignore files except that submodules are actually tracked. I think we should do this part in a different series, as everybody seems to agree that this should be fixed that way and it has nothing to do with what is ignored in submodule history. So how about I put the two points above into a separate series? IMO, add and status belong together in this case. * Let diff always show submodule changes between revisions or between a revision and the index. Only worktree changes should be ignored with ignore=all. * Generally speaking: Make everything that displays diffs in history, diffs between revisions or between a revision and the index always show submodules changes (only the commit ids) even if a submodule is specified as ignore=all. I'm not so sure about that. Some scripts really want to ignore the history of submodules when comparing a rev to the index: git-filter-branch.sh: git diff-index -r --name-only --ignore-submodules $commit git-pull.sh:git diff-index --cached --name-status -r --ignore-submodules HEAD -- git-rebase--merge.sh: if ! git diff-index --quiet --ignore-submodules HEAD -- git-sh-setup.sh: if ! git diff-index --cached --quiet --ignore-submodules HEAD -- git-stash.sh: git diff-index --quiet --cached HEAD --ignore-submodules -- I didn't check each site in detail, but I suspect each ignore option was added on purpose to fix a problem. That means we still need all (at least when diffing rev-index). Unfortunately that area is not covered well in our tests, I only got breakage from the filter-branch tests when teaching all to only ignore work tree changes (see at the end on how I did that). Well all hits are on diff-index which is plumbing. If it is required for some (internal) scripts to completely ignore submodules I think it is ok to do so just for plumbing with a commandline option like that. But I am not sure whether this should actually be configurable. So I'm currently in favor of adding a new worktree-value which will only ignore the work tree changes of submodules, which seems just what the floating submodule use case needs. But it looks like we need to keep all. But that will just add more complexity to the already complex topic of submodules. There are already enough possibilities to get confused with submodules. I would like to avoid making it more complex. From the feedback we get now from Sergey I take that not many users have actually been using the 'all' option. Otherwise there would have been more complaints. So the only thing we have to worry about are scripts and those we could cover with plumbing commands. * If ignore=all for a submodule and a diff would usually involve the worktree we will show the diff of the commit ids between the current index and the requested revision. I agree if we make that ignore=worktree. I do think that it is a good thing to make what git add . does and what git status . reports consistent, and git add . that does not add everything may be a good step in that direction Yup, as written above I'd propose to start with that too. (another possible solution may be to admit that ignore=all was a mistake and remove that special case altogether, so that git status will always report a submodule that does not match what is in the HEAD and/or index). No, looking at the git-scripts that use it together with diff-index it wasn't a mistake. But we might be missing a less drastic option ;-) Well, as said above diff-index is plumbing and as such allowed to do non-user friendly stuff. For all the other commands I propose to never hide stuff from the user. E.g. take update-index there you can actually mark any index entry as assume unchanged which will make git stop checking it for changes (like the submodule.name.ignore=all setting for submodules). IMO that is a
Re: Re: [RFC/WIP PATCH 3/4] teach add -f option for ignored submodules
On Fri, Dec 06, 2013 at 03:10:31PM -0800, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
diff --git a/builtin/add.c b/builtin/add.c
index 2d0d2ef..d6cab7f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -550,6 +569,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
for (i = 0; i pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
+ char path_copy[PATH_MAX];
if (!seen[i]
((pathspec.items[i].magic
(PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
@@ -562,6 +582,9 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_(pathspec '%s' did not match any
files),
pathspec.items[i].original);
}
+ normalize_path_copy(path_copy, path);
+ if (is_ignored_submodule(path_copy))
+ string_list_insert(ignored_submodules, path);
}
free(seen);
}
@@ -583,6 +606,8 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
update_files_in_cache(prefix, pathspec, update_data);
exit_status |= !!update_data.add_errors;
+ if (!ignored_too ignored_submodules.nr)
+ die_ignored_submodules(ignored_submodules);
Why is this done so late in the process? Shouldn't it be done
immediately after we have finished iterating over the pathspecs,
checking with is_ignored_submodule() and stuffing them into
ignored_submodules string list, not waiting for plugging bulk
checkin or updating paths already tracked in the index?
There was no specific reason. I just imitated the codepath for new
files (which will die in add_files() if they are ignored). This can be
moved further up. Will do so.
Cheers Heiko
--
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] t5541: Improve push test
Torsten Bögershausen tbo...@web.de writes: The old log-line looked like this: + 9d498b0...8598732 master - master (forced update) And the new one like this: 9d498b0..8598732 master - master - Loosen the grep pattern by not demanding (forced update) Hmm, what is the reason for the change the output? The output this piece is testing is the result of this: git push origin master:retsam echo change changed path2 git commit -a -m path2 --amend # push master too; this ensures there is at least one ''push'' command to # the remote helper and triggers interaction with the helper. test_must_fail git push -v origin +master master:retsam output 21' This is run inside test_repo_clone, which has /smart/test_repo.git as its origin, which in turn has 'master' branch (and nothing else). It - pushes master to another branch retsam; - amends its 'master'; - attempts to push the updated master to force-update master, and also retsam without forcing. The latter needs to be forced to succeed, and that is why we expect it to fail. If the output from the push process says + 9d498b0...8598732 master - master (forced update) ! [rejected]master - retsam (non-fast-forward) error: failed to push some refs to '../test_repo_copy/' I think that is a good thing to do, no? After all, that is what we show with Git native transports. Is this patch merely matching a test to a broken behaviour of some sort? Puzzled... -- 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: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all
Jens Lehmann jens.lehm...@web.de writes: I didn't check each site in detail, but I suspect each ignore option was added on purpose to fix a problem. That means we still need all (at least when diffing rev-index). Unfortunately that area is not covered well in our tests, I only got breakage from the filter-branch tests when teaching all to only ignore work tree changes (see at the end on how I did that). So I'm currently in favor of adding a new worktree-value which will only ignore the work tree changes of submodules, which seems just what the floating submodule use case needs. Could you help me clarify what it exactly mean to only ignore the work tree changes? Specifically, if I have a submodule directory whose (1) HEAD points at a commit that is the same as the commit that is recorded by the top-level's gitlink, (2) the index may or may not match HEAD, and (3) the working tree contents may or may not match the index or the HEAD, does it only have the work tree changes? If the HEAD in the submodule directory is different from the commit recorded by the top-level's gitlink, but the index and the working tree match that different HEAD, I am guessing that it no longer is with only the work tree changes and shown as modified. If that is the suggestion, it goes back to the very original Git submodule behavour where we compare $submoduledir/.git/HEAD with the commit object name expected by the top-level and say the submodule does not have any change when and only when these two object names are the same, which sounds like a very sensible default behaviour (besides, it is very cheap to check ;-). -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote: Junio C Hamano gits...@pobox.com writes: * jk/pack-bitmap (2013-11-18) 22 commits [...] Peff can decide if he wants to reroll with my nits or not; either way I'm all for moving it forward and aiming for one of the next releases. I'm going to be a bit slow this week, as I'm traveling in China. I have at least one more local fix queued up (one of the re-rolls introduced a use-after-free). Your comments make sense to me, though some of them are if this is not too hard, and I haven't looked yet to see how hard some of the requisite refactoring would be. So expect at least one more re-roll, and I'll try to incorporate your comments. Thanks for giving it a careful reading. As an aside, we have been running the last version sent to the list (modulo the fix I mentioned above) on github.com for a week or two (previously we were running the old, based-on-v1.8.4 version). So it is getting exercised. -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: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
On Fri, Dec 06, 2013 at 03:48:46PM +, Charlie Dyson wrote: gitmodules(5) states that submodule.$name.update should be defined in .gitmodules. However in cmd_update() in git-submodule.sh, git config is used with -f .gitmodules. Consequently this flag is only respected in .git/config Tested against: 1.8.2.1 [sorry! I've checked the relevant bit of source and it's the same] Steps to reproduce: $ git init $ git submodule add -b master someproject $ git config -f .gitmodules --add submodule.someproject.update merge $ # Go to someproject and commit something $ git submodule update --remote The latter does not perform a merge, and behaviour is visibly different to adding --merge. This is because of histerical reasons. When submodules were first implemented the notion was that .gitmodules should only be used as a starting point to initialise the configuration in .git/config when init was called. This notion has changed in a way that only the url (by that the name) should be copied on init. The default for everything else should come from .gitmodules or gits own default. The update configuration option was implemented before we realized that. So currently it is still copied when submodule init is called. The main reason is that we have not found the time to change that. I would submit a patch but I'm not completely sure what the behaviour would be - simply adding -f .gitmodules would hurt users that have adopted the practice of specifying their update preference in .git/config. Well .gitmodules should only be the fallback if there is nothing in .git/config. Perhaps the right thing to do is read from .git/config and fall back to .gitmodules using get_submodule_config(). Yes IMO that is the right thing to do. If you implement it that way (and remove the automatic copying on init) you should not break peoples expectations. So if you want to go ahead please send a patch. Cheers Heiko -- 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: Re: Publishing filtered branch repositories - workflow / recommendations?
On Fri, Dec 06, 2013 at 02:40:15PM -0500, Martin Langhoff wrote: On Fri, Dec 6, 2013 at 3:48 AM, Jens Lehmann jens.lehm...@web.de wrote: Right you are, we need tutorials for the most prominent use cases. In the meantime, are there any hints? Emails on this list showing a current smart workflow? Blog posts? Notes on a wiki? None that I know of mainly because we have not yet reached the goal we are aiming at. Maybe we should write something, A few points from $dayjob that come to my mind: * A submodule commit is only allowed to be merged into master in a superproject commit if it is merged into master (or a stable branch) in the submodule. That way you ensure that any submodules commits that are tracked in a superproject are contained in each other and can be cleanly merged. (no rewinds, one commit contains the other) * Submodule should be selfcontained (i.e. if its a library have tests that use the code you implement). That way changes in the submodule can be made independent from the superproject * If a submodule needs another submodule have them side by side instead of one inside another. See the next point for explanation. * Only one depth of recursion for submodules. Even though intuition tell you that if some submodule needs another it should contain the other its IMO not wise to do so. There will be times when some other submodule needs the same submodule that is contained in the other and then you end up with two copies of the same code. My suggestion: Let the superproject bundle all the dependencies between modules. * Submodules are a good solution for shared code where the dependency goes superproject needs submodule. If you divide code into submodules because of access control and the dependency is actually that the submodule needs the superproject it works but is less than optimal. Thats what I can quickly suggest and probably far from complete. Cheers Heiko -- 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] rebase: use reflog to find common base with upstream
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
Add a '--fork-point' argument to git-rebase that can be used to trigger
this behaviour. This option is turned on by default if no non-option
arguments are specified on the command line, otherwise we treat an
upstream specified on the command-line literally.
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Mon, Dec 09, 2013 at 08:40:08PM +, John Keeping wrote:
On Mon, Dec 09, 2013 at 12:11:50PM -0800, Junio C Hamano wrote:
Do you mean
git rebase no other arguments
which we interpret as rebase the current branch on @{u}, and it
should behave as if the command was run like so:
git rebase --fork-point @{u}
If that is what you suggest, I certainly can buy that. Those who
want to disable the automation can explicitly say
git rebase @{u}
and rebase the current exactly on top of the named commit (e.g. the
current value of refs/remotes/origin/master or whatever remote-tracking
branch you forked from).
Yes, that's what I meant; the first non-option argument to git rebase
is called upstream in the manpage (and throughout the code). So if
no other arguments means no non-option arguments then that's
exactly what I meant.
Here's an updated patch that adds a --fork-point option to git-rebase,
with the behaviour described above.
Documentation/git-rebase.txt | 10 ++
git-rebase.sh| 19 +++
t/t3400-rebase.sh| 6 --
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 94e07fd..2889be6 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -324,6 +324,16 @@ fresh commits so it can be remerged successfully without
needing to revert
the reversion (see the
link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for
details).
+--fork-point::
+--no-fork-point::
+ Use 'git merge-base --fork-point' to find a better common ancestor
+ between `upstream` and `branch` when calculating which commits have
+ have been introduced by `branch` (see linkgit:git-merge-base[1]).
++
+If no non-option arguments are given on the command line, then the default is
+`--fork-point @{u}` otherwise the `upstream` argument is interpreted literally
+unless the `--fork-point` option is specified.
+
--ignore-whitespace::
--whitespace=option::
These flag are passed to the 'git apply' program
diff --git a/git-rebase.sh b/git-rebase.sh
index 226752f..7185dc8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -14,6 +14,7 @@ git-rebase --continue | --abort | --skip | --edit-todo
v,verbose! display a diffstat of what changed upstream
q,quiet! be quiet. implies --no-stat
autostash! automatically stash/stash pop before and after
+fork-point use 'merge-base --fork-point' to refine upstream
onto=! rebase onto given branch instead of upstream
p,preserve-merges! try to recreate merges instead of ignoring them
s,strategy=! use the given merge strategy
@@ -66,6 +67,7 @@ verbose=
diffstat=
test $(git config --bool rebase.stat) = true diffstat=t
autostash=$(git config --bool rebase.autostash || echo false)
+fork_point=auto
git_am_opt=
rebase_root=
force_rebase=
@@ -260,6 +262,12 @@ do
--no-autosquash)
autosquash=
;;
+ --fork-point)
+ fork_point=t
+ ;;
+ --no-fork-point)
+ fork_point=
+ ;;
-M|-m)
do_merge=t
;;
@@ -437,6 +445,8 @@ then
error_on_missing_default_upstream rebase rebase \
against git rebase branch
fi
+
+ test $fork_point = auto fork_point=t
;;
*) upstream_name=$1
shift
@@ -522,6 +532,15 @@ case $# in
;;
esac
+if test $fork_point = t
+then
+ new_upstream=$(git merge-base --fork-point $upstream_name
$switch_to)
+ if test -n $new_upstream
+ then
+ upstream=$new_upstream
+ fi
+fi
+
if test $autostash = true ! (require_clean_work_tree) 2/dev/null
then
stash_sha1=$(git stash create autostash) ||
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index ebf93b0..998503d 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -134,12 +134,14 @@ test_expect_success 'fail when
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 09.12.2013 21:08, schrieb Jonathan Nieder:
Karsten Blees wrote:
GCC supports __packed__ as of 2.3 (1992), so any other compilers
that copied the __attribute__ feature probably won't complain.
Alas, it looks like HP C doesn't support __packed__ (not that I
care much about HP C):
http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes
Thanks for the link
Maybe a macro expanding to __attribute__((aligned(1))) on the fields
would work? The same macro could expand to __declspec(align(1)) in
the MSVC build.
But alignment is not the same as packing. We still want the structure to be
8-byte aligned (i.e. variables of the type should start at 8-byte boundaries).
We just don't want the size of the structure to be padded to a multiple of 8,
so that we can extend it without penalty. (Besides, __attribute__((aligned)) /
__declspec(align) can only _increase_ the alignment, so aligned(1) would have
no effect).
Googling some more, I believe the most protable way to achieve this via
'compiler settings' is
#pragma pack(push)
#pragma pack(4)
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
#pragma pack(pop)
This is supported by at least GCC, MSVC and HP (see your link). The downside is
that we cannot use macros (in git-compat-util.h) to emit #pragmas. But we
wouldn't have to, as compilers aren't supposed to barf on unknown #pragmas.
However, considering the portability issues, the macro solution (injecting just
the two fields instead of a struct) becomes more and more attractive in my
mind...
Karsten
--
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: A couple of rebase --autosquash proposals
I had not previously noticed commit --fixup, so that is something useful I have learned from this thread, thanks. The workflow here can be summarized as I have an initial commit and subsequent, review-generated commits, that I'd like to share on a review-branch with proper commit-log comments, but also pre-marked for future --autosquash. So when the review is completed, I can auto squash/fixup all the review-generated commits and rebase onto origin/master at the same time. I find this more appealing than continually pushing rebased branches to colleagues, as the history is lost and it is hard to review incremental changes. I can live with it as it is: I just use rebase -i and change all review-generated commits pick - r as if autosquash didn't exist. It's just that when I first tried-out fixup!, I mistakenly thought that I could use the first line as the special syntax, and use following-lines as annotation - which is not the case, but I thought it might be worth suggesting here. Brett On 10 December 2013 07:20, Junio C Hamano gits...@pobox.com wrote: Johannes Sixt j.s...@viscovery.net writes: Am 12/9/2013 3:23, schrieb Brett Randall: * fixup! or squash! on it's own would default to fixing-up the previous commit (or result of previous step of rebase if that was a squash/fixup). Why would you want that? To fixup the previous commit, just use 'git commit --amend'. What am I missing? When you are not absolutely sure if the amend is a good thing to do. Then work work work git commit --fixup HEAD work work work git commit --fixup HEAD^ work work work git commit --fixup HEAD^^ ... git rebase --autosquash -i ... may become a good way to polish a single commit. -- 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: [RFC/WIP PATCH] implement reading of submodule .gitmodules configuration into cache
Heiko Voigt hvo...@hvoigt.net writes:
This submodule configuration cache allows us to lazily read .gitmodules
configurations by commit into a runtime cache which can then be used to
easily lookup values from it. Currently only the values for path or name
are stored but it can be extended for any value needed.
...
Thanks.
diff --git a/submodule-config-cache.c b/submodule-config-cache.c
new file mode 100644
index 000..7253fad
--- /dev/null
+++ b/submodule-config-cache.c
@@ -0,0 +1,96 @@
+#include cache.h
+#include submodule-config-cache.h
+#include strbuf.h
+#include hash.h
+
+void submodule_config_cache_init(struct submodule_config_cache *cache)
+{
+ init_hash(cache-for_name);
+ init_hash(cache-for_path);
+}
+
+static int free_one_submodule_config(void *ptr, void *data)
+{
+ struct submodule_config *entry = ptr;
+
+ strbuf_release(entry-path);
+ strbuf_release(entry-name);
+ free(entry);
+
+ return 0;
+}
+
+void submodule_config_cache_free(struct submodule_config_cache *cache)
+{
+ /* NOTE: its important to iterate over the name hash here
+ * since paths might have multiple entries */
Style (multi-line comments).
This is interesting. I wonder what the practical consequence is to
have a single submodule bound to the top-level tree more than once.
Updating from one of the working tree will make the other working
tree out of sync because the ultimate location of the submodule
directory pointed at by the two .git gitdirs can only have a single
HEAD, be it detached or on a branch, and a single index.
Not that the decision to enforce that names are unique in the
top-level .gitmodules, and follow that decision in this part of the
code to be defensive (not rely on the one submodule can be bound
only once to a top-level tree), but shouldn't such a configuration
to have a single submodule bound to more than one place in the
top-level tree be forbidden?
+ for_each_hash(cache-for_name, free_one_submodule_config, NULL);
+ free_hash(cache-for_path);
+ free_hash(cache-for_name);
+}
+
+static unsigned int hash_sha1_string(const unsigned char *sha1, const char
*string)
+{
+ int c;
+ unsigned int hash, string_hash = 5381;
+ memcpy(hash, sha1, sizeof(hash));
+
+ /* djb2 hash */
+ while ((c = *string++))
+ string_hash = ((string_hash 5) + hash) + c; /* hash * 33 + c
*/
Hmm, the comment and the code does not seem to match in math here...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
Heiko Voigt hvo...@hvoigt.net writes: This notion has changed in a way that only the url (by that the name) should be copied on init. The default for everything else should come from .gitmodules or gits own default. I think you need to be a bit careful here. I do not think everything should blindly default to .gitmodules (otherwise there are obvious security implications as well as usability ones). The update configuration option was implemented before we realized that. So currently it is still copied when submodule init is called. The main reason is that we have not found the time to change that. And 'update', as it allows any custom way to run it, is unlikely to be allowed to be used blindly from .gitmodules, no? -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees wrote:
(Besides, __attribute__((aligned)) / __declspec(align) can only
_increase_ the alignment, so aligned(1) would have no effect).
Good catch.
Googling some more, I believe the most protable way to achieve this
via 'compiler settings' is
#pragma pack(push)
#pragma pack(4)
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
#pragma pack(pop)
This is supported by at least GCC, MSVC and HP (see your link). The
downside is that we cannot use macros (in git-compat-util.h) to emit
#pragmas. But we wouldn't have to, as compilers aren't supposed to
barf on unknown #pragmas.
Technically this can be done using macros:
#if (gcc)
# define BEGIN_PACKED _Pragma(pack(push,4))
# define END_PACKED _Pragma(pack(pop))
#elif (msvc)
# define BEGIN_PACKED __pragma(pack(push,4))
# define END_PACKED __pragma(pack(pop))
#else
/* Just tolerate a little padding. */
# define BEGIN_PACKED
# define END_PACKED
#end
Then you can do:
BEGIN_PACKED
struct hashmap_entry {
...
};
END_PACKED
Whether that's nicer or uglier than the alternatives I leave to you.
;-)
Thanks,
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: Publishing filtered branch repositories - workflow / recommendations?
Heiko Voigt hvo...@hvoigt.net writes: On Fri, Dec 06, 2013 at 02:40:15PM -0500, Martin Langhoff wrote: On Fri, Dec 6, 2013 at 3:48 AM, Jens Lehmann jens.lehm...@web.de wrote: Right you are, we need tutorials for the most prominent use cases. In the meantime, are there any hints? Emails on this list showing a current smart workflow? Blog posts? Notes on a wiki? None that I know of mainly because we have not yet reached the goal we are aiming at. Maybe we should write something, A few points from $dayjob that come to my mind: * A submodule commit is only allowed to be merged into master in a superproject commit if it is merged into master (or a stable branch) in the submodule. That way you ensure that any submodules commits that are tracked in a superproject are contained in each other and can be cleanly merged. (no rewinds, one commit contains the other) I think this is closely related to Martin's list of wishes we earlier saw in the thread: remind the user to push necessary submodule tip before the top-level commit that needs that commit in the submodule is pushed out. Giving projects a way to implement such a policy decision would be good, and having a default policy, if we can find one that would be reasonable for any submodule users, would be even better. Would adding a generic pre-push hook at the top-level sufficient for that kind of thing, I have to wonder. -- 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] pull: use merge-base --fork-point when appropriate
John Keeping wrote: Since commit d96855f (merge-base: teach --fork-point mode, 2013-10-23) we can replace a shell loop in git-pull with a single call to git-merge-base. So let's do so. Signed-off-by: John Keeping j...@keeping.me.uk --- git-pull.sh | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) Yay! Looks good. -- 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
Refspec wildcards for remotes require trailing slash
Hi I came across this issue the other day while trying to optimise a fetch to a repo with release branches named release-X.X.X. I wanted to just fetch just those branches but because of the trailing slash rule I couldn't. The following StackOverflow post has details of the issue: http://stackoverflow.com/questions/20450003/is-it-possible-to-use-filters-in-refspec-in-places-other-than-directory-namespac The trailing slash rule appears to be introduced in 46220ca and further tightened in b2a5627 Before I go ahead and look at what needs to be done for a patch I thought it would be polite to ask if there is any reasoning behind the trailing slash rule that I'm missing? Or if you are interested in changing this behavior at all. Cheers Monte-- 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] Additional git-archive tests
On 5 Dec 2013, at 11:52, Junio C Hamano gits...@pobox.com wrote: Nick Townsend nick.towns...@mac.com writes: Interplay between paths specified in three ways now tested: * After a : in the tree-ish, * As a pathspec in the command, * By virtue of the current working directory Note that these tests are based on the behaviours as found in 1.8.5. They may not be intentional. They were developed to regression test enhancements made to parse_treeish_arg() in archive.c In other words, are all these new tests expected to pass? Sorry about that - the first four tests do pass with 1.8.5, the last three tests do not currently pass. I believe they should pass and with my reworked git-archive patch (similar to the previous one)they do. (though I removed that update from this set pending the discussion). Currently 5 and 6 fail with the message: ‘fatal: current working directory is untracked’ and number 7 fails with: ‘fatal: Not a valid object name' My cursory read of parse_treeish_arg() in archive.c is: - It reads the given object with get_sha1(), checking if it is a commit-ish or tree-ish to decide if it wants to add the pax header to record the commit object name; - It parses the tree object; - If run from a subdirectory, attempts to grab the prefix (i.e. the path to the current subdirectory---in the tests you added, they are all a/) out of that tree object (it errors out if it can't); and then - It archives the tree object. So I do not think it is expected to accept tree object names with the HEAD:path style syntax, if the user expects a predictable and consistent result. The third step above attempts to make sure that you name a tree-ish that corresponds to the top-level of the project, i.e. with no path. That’s not quite right - paths do work from the root directory - see tests. It was this very useful capability that I sought to add and generalized the code to do. What seems to be supported are: cd a git archive HEAD ;# archives HEAD:a tree cd a git archive HEAD -- b ;# archives a/b/ part of HEAD:a as b/ Specifically, it appears that HEAD:./b, HEAD:b etc. are not designed to work, at least to me. Clearly when you specify them today they don’t work, but I believe they should work. I am not saying that these should _not_ work, but it is unclear what it means to work. For example, what should this do? cd a git archive HEAD:./b $pathspec I think we can clear this up by documenting the cases and choosing sensible behaviour _for git-archive_ ie. what people might expect. Here is a suggestion: a. The pathspec is used as a selector to include things in the archive. it is applied after the cwd and treeish selection. b. The current working directory (if present) prefixes a path in the object if one is present. c. The path in the object (if present) is prefixed by the cwd (if present) and used to select items for archiving. However the composite path so created *is not present* in the archive - ie. the leading components are stripped. (This is very useful behaviour for creating archives without leading paths) d. As currently the —prefix option (not the prefix from setup_git_directory) is prepended to all entries in the archive. These correspond to the use cases that I have for git archive - extracting all or part of a multiple submodule tree into a tgz file, with or without leading directories. The extended SHA-1 expression HEAD:./b in the subdirectory a/ is interpreted by get_sha1_with_context_1() to be the name of the tree object at path a/b in the commit HEAD. Further, you are giving a pathspec while in a subdirectory a/ to the command. What should that pathspec be relative to? In a normal Git command, the pathspec always is relative to the current subdirectory, so, the way to learn about the tree object a/b/c in the HEAD while in subdirectory a/ would be: cd a git ls-tree HEAD b/c But what should the command line for archive to grab HEAD:a/b/c be? It feels wrong to say: cd a git archive HEAD:./b b/c It’s clear to me that if you are in a subdirectory, that is an implicit prefix on the ./b so you retrieve HEAD:a/b which then archives everything in b without the leading a/b. The pathspec is wrong (including the b) and this archive is empty. and it also feels wrong to say cd a git archive HEAD:./b c That looks fine to me given the rules suggested above. Also git-parse manages to return the correct thing in this case, so I’d expect this to work. No matter what we would do, we should behave consistently with this case: treeish=$(git rev-parse HEAD:a/b) cd a git archive $treeish -- $pathspec Well this will fail both in the existing case (‘fatal: current working directory is untracked’) and with the scheme proposed above (‘fatal: invalid object name $treeish:a/‘) so take the pathspec relative to the tree when the treeish was given with 'treeish:path' syntax,
