Re: Git merge: conflict is expected, but not detected
From the perspective of topic there had been no change to the definition of bar(), hence there was no change to contribute to the eventual merge with master. One way to avoid this kind of problem is to avoid making (or cherry-picking) the same change on different branches, but instead use a merge of a branch with a common base to implement changes needed on multiple branches. So, assuming you recognized the need to delete bar() from both topic and master, create a new branch from the merge-base of topic and master and delete bar() in that branch. Then merge this branch into both topic and master. If you subsequently decide to revert the removal of bar() on topic then when you decide to merge topic back into master, git will see that the removal branch has been merged into both branches and will see the subsequent revert on topic as a change that needs to be merged and you will get the result you are looking for. So, as a general rule of thumb, try to avoid making the same change on two different branches and instead factor out a change needed in multiple places into a separate branch which is then merged into the branches that need iit. On Sat, Nov 30, 2013 at 1:26 AM, Evgeniy Ivanov lolkaanti...@gmail.com wrote: Hi! Let's say I have two identical branches: master and topic. In master I remove some code, i.e. function bar(). In topic I do the same (commit) and after some time I realize I need bar() and revert previous commit with removal. So I end with master with no bar() and topic with bar() in its original state. When I merge I get code without bar() and no merge conflict (recursive or resolve strategies). Is it possible to detect such situations as conflicts? When bar() is C++ virtual there is no possibility to catch this with compiler. Please, CC me since I'm not subscribed. Thanks in advance! -- Cheers, Evgeniy -- 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 -- 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-svn troubles with calendarserver SVN repo
I have seen this behaviour, though never determined the root cause .Probably the simplest thing you can do without access to the server is to put your git svn fetch into a bash while loop, like so: while ! git svn fetch; do :; done; jon On Sat, Nov 30, 2013 at 10:14 AM, Matěj Cepl mc...@redhat.com wrote: Hi, I am trying to git-svn clone https://svn.calendarserver.org/repository/calendarserver/CalendarServer/ and I cannot say I am much succesful. Every couple of hundred of commits I get this: RA layer request failed: REPORT of '/repository/calendarserver/!svn/vcc/default': could not connect to server (https://svn.calendarserver.org) at /usr/local/share/perl5/Git/SVN/Ra.pm line 290. and git-svn stops. When restarting git svn fetch, it fetches another couple of hundred commits and then fails again. Given that there are 11000+ commits just in the trunk, I am afraid of a long long night. Did anybody managed to clone this repo? Or is there some way how to make git-svn more patient (this error means the SVN server is a bit flakey, right)? Best, Matěj -- http://www.ceplovi.cz/matej/, Jabber: mc...@ceplovi.cz GPG Finger: 89EF 4BC6 288A BF43 1BAB 25C3 E09F EF25 D964 84AC A day without sunshine is like night. -- 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 12/24] read-cache: read index-v5
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
--- a/cache.h
+++ b/cache.h
@@ -132,11 +141,17 @@ struct cache_entry {
char name[FLEX_ARRAY]; /* more */
};
+#define CE_NAMEMASK (0x0fff)
CE_NAMEMASK is redefined in read-cache-v2.c in read-cache: move index
v2 specific functions to their own file. My gcc is smart enough to
see the two defines are about the same value and does not warn me. But
we should remove one (likely this one as I see no use of this macro
outside read-cache-v2.c)
#define CE_STAGEMASK (0x3000)
#define CE_EXTENDED (0x4000)
#define CE_VALID (0x8000)
+#define CE_SMUDGED (0x0400) /* index v5 only flag */
#define CE_STAGESHIFT 12
+#define CONFLICT_CONFLICTED (0x8000)
+#define CONFLICT_STAGESHIFT 13
+#define CONFLICT_STAGEMASK (0x6000)
+
/*
* Range 0x in ce_flags is divided into
* two parts: in-memory flags and on-disk ones.
diff --git a/read-cache-v5.c b/read-cache-v5.c
new file mode 100644
index 000..9d8c8f0
--- /dev/null
+++ b/read-cache-v5.c
+static int read_index_v5(struct index_state *istate, void *mmap,
+unsigned long mmap_size, struct filter_opts *opts)
+{
+ unsigned int entry_offset, foffsetblock, nr = 0, *extoffsets;
+ unsigned int dir_offset, dir_table_offset;
+ int need_root = 0, i;
+ uint32_t *offset;
+ struct directory_entry *root_directory, *de, *last_de;
+ const char **paths = NULL;
+ struct pathspec adjusted_pathspec;
+ struct cache_header *hdr;
+ struct cache_header_v5 *hdr_v5;
+
+ hdr = mmap;
+ hdr_v5 = ptr_add(mmap, sizeof(*hdr));
+ istate-cache_alloc = alloc_nr(ntohl(hdr-hdr_entries));
+ istate-cache = xcalloc(istate-cache_alloc, sizeof(struct
cache_entry *));
+ extoffsets = xcalloc(ntohl(hdr_v5-hdr_nextension), sizeof(int));
+ for (i = 0; i ntohl(hdr_v5-hdr_nextension); i++) {
+ offset = ptr_add(mmap, sizeof(*hdr) + sizeof(*hdr_v5));
+ extoffsets[i] = htonl(*offset);
+ }
+
+ /* Skip size of the header + crc sum + size of offsets to extensions
+ size of offsets */
+ dir_offset = sizeof(*hdr) + sizeof(*hdr_v5) +
ntohl(hdr_v5-hdr_nextension) * 4 + 4
+ + (ntohl(hdr_v5-hdr_ndir) + 1) * 4;
+ dir_table_offset = sizeof(*hdr) + sizeof(*hdr_v5) +
ntohl(hdr_v5-hdr_nextension) * 4 + 4;
+ root_directory = read_directories(dir_offset, dir_table_offset,
+ mmap, mmap_size);
+
+ entry_offset = ntohl(hdr_v5-hdr_fblockoffset);
+ foffsetblock = dir_offset;
+
+ if (opts opts-pathspec opts-pathspec-nr) {
+ paths = xmalloc((opts-pathspec-nr + 1)*sizeof(char *));
+ paths[opts-pathspec-nr] = NULL;
Put this statement here
GUARD_PATHSPEC(opts-pathspec,
PATHSPEC_FROMTOP |
PATHSPEC_MAXDEPTH |
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE);
This says the mentioned magic is safe in this code. New magic may or
may not be and needs to be checked (soonest by me, I'm going to add
negative pathspec and I'll need to look into how it should be handled
in this code block).
+ for (i = 0; i opts-pathspec-nr; i++) {
+ char *super = strdup(opts-pathspec-items[i].match);
+ int len = strlen(super);
You should only check as far as items[i].nowildcard_len, not strlen().
The rest could be wildcards and stuff and not so reliable.
+ while (len super[len - 1] == '/' super[len - 2]
== '/')
+ super[--len] = '\0'; /* strip all but one
trailing slash */
+ while (len super[--len] != '/')
+ ; /* scan backwards to next / */
+ if (len = 0)
+ super[len--] = '\0';
+ if (len = 0) {
+ need_root = 1;
+ break;
+ }
+ paths[i] = super;
+ }
And maybe put the comment FIXME: consider merging this code with
create_simplify() in dir.c somewhere. It's for me to look for things
to do when I'm bored ;-)
+ }
+
+ if (!need_root)
+ parse_pathspec(adjusted_pathspec, PATHSPEC_ALL_MAGIC,
PATHSPEC_PREFER_CWD, NULL, paths);
I would go with PATHSPEC_PREFER_FULL instead of _CWD as it's safer.
Looking only at this function without caller context, it's hard to say
if _CWD is the right choice.
+
+ de = root_directory;
+ last_de = de;
This statement is redundant. last_de is only used in one code block
below and it's always re-initialized before entering the loop to skip
subdirs.
+ while (de) {
+ if (need_root ||
+
Re: [PATCH v4 09/24] ls-files.c: use index api
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
@@ -447,6 +463,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
struct dir_struct dir;
struct exclude_list *el;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct filter_opts *opts = xmalloc(sizeof(*opts));
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
N_(paths are separated with NUL character),
@@ -512,9 +529,6 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
prefix_len = strlen(prefix);
git_config(git_default_config, NULL);
- if (read_cache() 0)
- die(index file corrupt);
-
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
el = add_exclude_list(dir, EXC_CMDL, --exclude option);
@@ -550,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
+ if (!with_tree !needs_trailing_slash_stripped()) {
+ memset(opts, 0, sizeof(*opts));
+ opts-pathspec = pathspec;
+ opts-read_staged = 1;
+ if (show_resolve_undo)
+ opts-read_resolve_undo = 1;
+ if (read_cache_filtered(opts) 0)
+ die(index file corrupt);
+ } else {
+ if (read_cache() 0)
+ die(index file corrupt);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
So we ran parse_pathspec() once (not shown in the context), if found
trailing slashes, we read full cache and rerun parse_pathspec()
because the index is not loaded on the first run.
This is fine. Just a note for future improvement: as _SLASH_CHEAP only
needs to look at a few entries with cache_name_pos(), we could take
advantage of v5 to peek individual entries (or in v2, load full cache
first). Nothing needs to be done now, I think we have not decided
whether to combine _SLASH_CHEAP and _SLASH_EXPENSIVE into one.
+ }
+
/* Find common prefix for all pathspec's */
max_prefix = common_prefix(pathspec);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gettext.c: only work around the vsnprintf bug on glibc 2.17
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/gettext.c b/gettext.c index 71e9545..91e679d 100644 --- a/gettext.c +++ b/gettext.c @@ -18,6 +18,13 @@ #endif #endif +#ifdef USE_GLIBC +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif Defining a feature test macro after any system header is included is undefined. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- 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 23/24] POC for partial writing
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
This makes update-index use both partial reading and partial writing.
Partial reading is only used no option other than the paths is passed to
the command.
This passes the test suite,
Just checking, the test suite was run with TEST_GIT_INDEX_VERSION=5, right?
but doesn't behave correctly when a write
fails. A log should be written to the lock file, in order to be able to
recover if a write fails.
From the API point of view this looks nice (you should have hidden
needs_write = 1 in cache_invalidate_path and change_cache_version
though) We could support partial file removal too by marking removed
files removed, but that impacts the reading code and may have bad
interaction with cache_invalidate_path/needs_rewrite. Probably not
worth the effort until someone shows us they remove stuff often.
---
builtin/update-index.c | 43 +++---
cache-tree.c | 13 +
cache-tree.h | 1 +
cache.h| 27 -
lockfile.c | 2 +-
read-cache-v2.c| 2 +
read-cache-v5.c| 154
-
read-cache.c | 30 ++
read-cache.h | 1 +
resolve-undo.c | 1 +
10 files changed, 237 insertions(+), 37 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8b3f7a0..69f0949 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -56,6 +56,7 @@ static int mark_ce_flags(const char *path, int flag, int
mark)
else
active_cache[pos]-ce_flags = ~flag;
cache_tree_invalidate_path(active_cache_tree, path);
+ the_index.needs_rewrite = 1;
active_cache_changed = 1;
return 0;
}
@@ -99,6 +100,8 @@ static int add_one_path(const struct cache_entry *old,
const char *path, int len
memcpy(ce-name, path, len);
ce-ce_flags = create_ce_flags(0);
ce-ce_namelen = len;
+ if (old)
+ ce-entry_pos = old-entry_pos;
fill_stat_cache_info(ce, st);
ce-ce_mode = ce_mode_from_stat(old, st-st_mode);
@@ -268,6 +271,7 @@ static void chmod_path(int flip, const char *path)
goto fail;
}
cache_tree_invalidate_path(active_cache_tree, path);
+ the_index.needs_rewrite = 1;
active_cache_changed = 1;
report(chmod %cx '%s', flip, path);
return;
@@ -706,15 +710,18 @@ static int reupdate_callback(struct parse_opt_ctx_t
*ctx,
int cmd_update_index(int argc, const char **argv, const char *prefix)
{
- int newfd, entries, has_errors = 0, line_termination = '\n';
+ int newfd, has_errors = 0, line_termination = '\n';
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
char set_executable_bit = 0;
struct refresh_params refresh_args = {0, has_errors};
int lock_error = 0;
+ struct filter_opts opts;
+ struct pathspec pathspec;
struct lock_file *lock_file;
struct parse_opt_ctx_t ctx;
+ int i, needs_full_read = 0;
int parseopt_state = PARSE_OPT_UNKNOWN;
struct option options[] = {
OPT_BIT('q', NULL, refresh_args.flags,
@@ -810,9 +817,23 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (newfd 0)
lock_error = errno;
- entries = read_cache();
- if (entries 0)
- die(cache corrupted);
+ for (i = 0; i argc; i++) {
+ if (!prefixcmp(argv[i], --))
+ needs_full_read = 1;
+ }
+ if (!needs_full_read) {
+ memset(opts, 0, sizeof(struct filter_opts));
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv + 1);
+ opts.pathspec = pathspec;
+ if (read_cache_filtered(opts) 0)
+ die(cache corrupted);
+ } else {
+ if (read_cache() 0)
+ die(cache corrupted);
+ }
/*
* Custom copy of parse_options() because we want to handle
@@ -862,6 +883,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
preferred_index_format,
INDEX_FORMAT_LB, INDEX_FORMAT_UB);
+ the_index.needs_rewrite = 1;
active_cache_changed = 1;
change_cache_version(preferred_index_format);
}
@@ -890,17 +912,22 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
}
if
Re: [PATCH v4 09/24] ls-files.c: use index api
Duy Nguyen pclo...@gmail.com writes:
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
@@ -447,6 +463,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
struct dir_struct dir;
struct exclude_list *el;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct filter_opts *opts = xmalloc(sizeof(*opts));
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
N_(paths are separated with NUL character),
@@ -512,9 +529,6 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
prefix_len = strlen(prefix);
git_config(git_default_config, NULL);
- if (read_cache() 0)
- die(index file corrupt);
-
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
el = add_exclude_list(dir, EXC_CMDL, --exclude option);
@@ -550,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const
char *cmd_prefix)
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
+ if (!with_tree !needs_trailing_slash_stripped()) {
+ memset(opts, 0, sizeof(*opts));
+ opts-pathspec = pathspec;
+ opts-read_staged = 1;
+ if (show_resolve_undo)
+ opts-read_resolve_undo = 1;
+ if (read_cache_filtered(opts) 0)
+ die(index file corrupt);
+ } else {
+ if (read_cache() 0)
+ die(index file corrupt);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
So we ran parse_pathspec() once (not shown in the context), if found
trailing slashes, we read full cache and rerun parse_pathspec()
because the index is not loaded on the first run.
This is fine. Just a note for future improvement: as _SLASH_CHEAP only
needs to look at a few entries with cache_name_pos(), we could take
advantage of v5 to peek individual entries (or in v2, load full cache
first). Nothing needs to be done now, I think we have not decided
whether to combine _SLASH_CHEAP and _SLASH_EXPENSIVE into one.
Yes that makes sense. Adding the ability to search for path entries
without reading the whole or part of the index was something I was
thinking about, but didn't have time to do so yet. I'll add this to my
list of possible future improvements.
+ }
+
/* Find common prefix for all pathspec's */
max_prefix = common_prefix(pathspec);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 12/24] read-cache: read index-v5
Duy Nguyen pclo...@gmail.com writes:
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
--- a/cache.h
+++ b/cache.h
@@ -132,11 +141,17 @@ struct cache_entry {
char name[FLEX_ARRAY]; /* more */
};
+#define CE_NAMEMASK (0x0fff)
CE_NAMEMASK is redefined in read-cache-v2.c in read-cache: move index
v2 specific functions to their own file. My gcc is smart enough to
see the two defines are about the same value and does not warn me. But
we should remove one (likely this one as I see no use of this macro
outside read-cache-v2.c)
Thanks for catching that, there's no need to have it here. I'll remove
it in the re-roll.
#define CE_STAGEMASK (0x3000)
#define CE_EXTENDED (0x4000)
#define CE_VALID (0x8000)
+#define CE_SMUDGED (0x0400) /* index v5 only flag */
#define CE_STAGESHIFT 12
+#define CONFLICT_CONFLICTED (0x8000)
+#define CONFLICT_STAGESHIFT 13
+#define CONFLICT_STAGEMASK (0x6000)
+
/*
* Range 0x in ce_flags is divided into
* two parts: in-memory flags and on-disk ones.
diff --git a/read-cache-v5.c b/read-cache-v5.c
new file mode 100644
index 000..9d8c8f0
--- /dev/null
+++ b/read-cache-v5.c
+static int read_index_v5(struct index_state *istate, void *mmap,
+unsigned long mmap_size, struct filter_opts *opts)
+{
+ unsigned int entry_offset, foffsetblock, nr = 0, *extoffsets;
+ unsigned int dir_offset, dir_table_offset;
+ int need_root = 0, i;
+ uint32_t *offset;
+ struct directory_entry *root_directory, *de, *last_de;
+ const char **paths = NULL;
+ struct pathspec adjusted_pathspec;
+ struct cache_header *hdr;
+ struct cache_header_v5 *hdr_v5;
+
+ hdr = mmap;
+ hdr_v5 = ptr_add(mmap, sizeof(*hdr));
+ istate-cache_alloc = alloc_nr(ntohl(hdr-hdr_entries));
+ istate-cache = xcalloc(istate-cache_alloc, sizeof(struct
cache_entry *));
+ extoffsets = xcalloc(ntohl(hdr_v5-hdr_nextension), sizeof(int));
+ for (i = 0; i ntohl(hdr_v5-hdr_nextension); i++) {
+ offset = ptr_add(mmap, sizeof(*hdr) + sizeof(*hdr_v5));
+ extoffsets[i] = htonl(*offset);
+ }
+
+ /* Skip size of the header + crc sum + size of offsets to extensions
+ size of offsets */
+ dir_offset = sizeof(*hdr) + sizeof(*hdr_v5) +
ntohl(hdr_v5-hdr_nextension) * 4 + 4
+ + (ntohl(hdr_v5-hdr_ndir) + 1) * 4;
+ dir_table_offset = sizeof(*hdr) + sizeof(*hdr_v5) +
ntohl(hdr_v5-hdr_nextension) * 4 + 4;
+ root_directory = read_directories(dir_offset, dir_table_offset,
+ mmap, mmap_size);
+
+ entry_offset = ntohl(hdr_v5-hdr_fblockoffset);
+ foffsetblock = dir_offset;
+
+ if (opts opts-pathspec opts-pathspec-nr) {
+ paths = xmalloc((opts-pathspec-nr + 1)*sizeof(char *));
+ paths[opts-pathspec-nr] = NULL;
Put this statement here
GUARD_PATHSPEC(opts-pathspec,
PATHSPEC_FROMTOP |
PATHSPEC_MAXDEPTH |
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE);
This says the mentioned magic is safe in this code. New magic may or
may not be and needs to be checked (soonest by me, I'm going to add
negative pathspec and I'll need to look into how it should be handled
in this code block).
Thanks, I'll add the statement in the re-roll.
+ for (i = 0; i opts-pathspec-nr; i++) {
+ char *super = strdup(opts-pathspec-items[i].match);
+ int len = strlen(super);
You should only check as far as items[i].nowildcard_len, not strlen().
The rest could be wildcards and stuff and not so reliable.
Ok, will change it in the re-roll.
+ while (len super[len - 1] == '/' super[len -
2] == '/')
+ super[--len] = '\0'; /* strip all but one
trailing slash */
+ while (len super[--len] != '/')
+ ; /* scan backwards to next / */
+ if (len = 0)
+ super[len--] = '\0';
+ if (len = 0) {
+ need_root = 1;
+ break;
+ }
+ paths[i] = super;
+ }
And maybe put the comment FIXME: consider merging this code with
create_simplify() in dir.c somewhere. It's for me to look for things
to do when I'm bored ;-)
Heh, thanks, will do.
+ }
+
+ if (!need_root)
+ parse_pathspec(adjusted_pathspec, PATHSPEC_ALL_MAGIC,
PATHSPEC_PREFER_CWD, NULL, paths);
I would go with PATHSPEC_PREFER_FULL instead of _CWD as it's safer.
Looking only at this function without caller context, it's hard to say
if _CWD is the right choice.
Ok, thanks, will change.
+
+ de = root_directory;
Re: [PATCH v4 23/24] POC for partial writing
Duy Nguyen pclo...@gmail.com writes:
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
This makes update-index use both partial reading and partial writing.
Partial reading is only used no option other than the paths is passed to
the command.
This passes the test suite,
Just checking, the test suite was run with TEST_GIT_INDEX_VERSION=5, right?
Yes, sure, I've been using that for finding problems in my approach.
but doesn't behave correctly when a write
fails. A log should be written to the lock file, in order to be able to
recover if a write fails.
From the API point of view this looks nice (you should have hidden
needs_write = 1 in cache_invalidate_path and change_cache_version
though).
Ok, thanks, I'll look into that, once I have time to make more than a
POC for this.
We could support partial file removal too by marking removed
files removed, but that impacts the reading code and may have bad
interaction with cache_invalidate_path/needs_rewrite. Probably not
worth the effort until someone shows us they remove stuff often.
Yes, right, I think it could be done, but I didn't take the time to look
into that for now. The reading code should actually be fine as it is,
as it already takes care of entries with the removed flag, but I'm not
sure about cache_tree_invalidate_path/needs_rewrite.
---
builtin/update-index.c | 43 +++---
cache-tree.c | 13 +
cache-tree.h | 1 +
cache.h| 27 -
lockfile.c | 2 +-
read-cache-v2.c| 2 +
read-cache-v5.c| 154
-
read-cache.c | 30 ++
read-cache.h | 1 +
resolve-undo.c | 1 +
10 files changed, 237 insertions(+), 37 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8b3f7a0..69f0949 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -56,6 +56,7 @@ static int mark_ce_flags(const char *path, int flag, int
mark)
else
active_cache[pos]-ce_flags = ~flag;
cache_tree_invalidate_path(active_cache_tree, path);
+ the_index.needs_rewrite = 1;
active_cache_changed = 1;
return 0;
}
@@ -99,6 +100,8 @@ static int add_one_path(const struct cache_entry *old,
const char *path, int len
memcpy(ce-name, path, len);
ce-ce_flags = create_ce_flags(0);
ce-ce_namelen = len;
+ if (old)
+ ce-entry_pos = old-entry_pos;
fill_stat_cache_info(ce, st);
ce-ce_mode = ce_mode_from_stat(old, st-st_mode);
@@ -268,6 +271,7 @@ static void chmod_path(int flip, const char *path)
goto fail;
}
cache_tree_invalidate_path(active_cache_tree, path);
+ the_index.needs_rewrite = 1;
active_cache_changed = 1;
report(chmod %cx '%s', flip, path);
return;
@@ -706,15 +710,18 @@ static int reupdate_callback(struct parse_opt_ctx_t
*ctx,
int cmd_update_index(int argc, const char **argv, const char *prefix)
{
- int newfd, entries, has_errors = 0, line_termination = '\n';
+ int newfd, has_errors = 0, line_termination = '\n';
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
char set_executable_bit = 0;
struct refresh_params refresh_args = {0, has_errors};
int lock_error = 0;
+ struct filter_opts opts;
+ struct pathspec pathspec;
struct lock_file *lock_file;
struct parse_opt_ctx_t ctx;
+ int i, needs_full_read = 0;
int parseopt_state = PARSE_OPT_UNKNOWN;
struct option options[] = {
OPT_BIT('q', NULL, refresh_args.flags,
@@ -810,9 +817,23 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (newfd 0)
lock_error = errno;
- entries = read_cache();
- if (entries 0)
- die(cache corrupted);
+ for (i = 0; i argc; i++) {
+ if (!prefixcmp(argv[i], --))
+ needs_full_read = 1;
+ }
+ if (!needs_full_read) {
+ memset(opts, 0, sizeof(struct filter_opts));
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv + 1);
+ opts.pathspec = pathspec;
+ if (read_cache_filtered(opts) 0)
+ die(cache corrupted);
+ } else {
+ if (read_cache() 0)
+ die(cache corrupted);
+ }
/*
* Custom copy of parse_options() because we want to handle
@@ -862,6 +883,7 @@ int
[PATCH v2] gettext.c: only work around the vsnprintf bug on glibc 2.17
Bug 6530 [1] causes git show v0.99.6~1 to fail with error your
vsnprintf is broken. The workaround avoids that, but it corrupts
system error messages in non-C locales.
The bug has been fixed since 2.17. If git is built with glibc, it can
know running libc version with gnu_get_libc_version() and avoid the
workaround on fixed versions. The workaround is also dropped for all
non-glibc systems.
Tested on Gentoo Linux, glibc 2.17, amd64.
[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
v2 removes USE_GLIBC and lets git-compat-util.h do the detection
gettext.c | 24
git-compat-util.h | 11 +++
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/gettext.c b/gettext.c
index 71e9545..772ab92 100644
--- a/gettext.c
+++ b/gettext.c
@@ -30,7 +30,7 @@ int use_gettext_poison(void)
#ifndef NO_GETTEXT
static const char *charset;
-static void init_gettext_charset(const char *domain)
+static void init_gettext_charset(const char *domain, int vsnprintf_broken)
{
/*
This trick arranges for messages to be emitted in the user's
@@ -99,9 +99,7 @@ static void init_gettext_charset(const char *domain)
$ LANGUAGE= LANG=de_DE.utf8 ./test
test: Kein passendes Ger?t gefunden
- In the long term we should probably see about getting that
- vsnprintf bug in glibc fixed, and audit our code so it won't
- fall apart under a non-C locale.
+ The vsnprintf bug has been fixed since 2.17.
Then we could simply set LC_CTYPE from the environment, which would
make things like the external perror(3) messages work.
@@ -112,21 +110,31 @@ static void init_gettext_charset(const char *domain)
1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530
2. E.g. Content-Type: text/plain; charset=UTF-8\n in po/is.po
*/
- setlocale(LC_CTYPE, );
+ if (vsnprintf_broken)
+ setlocale(LC_CTYPE, );
charset = locale_charset();
bind_textdomain_codeset(domain, charset);
- setlocale(LC_CTYPE, C);
+ if (vsnprintf_broken)
+ setlocale(LC_CTYPE, C);
}
void git_setup_gettext(void)
{
const char *podir = getenv(GIT_TEXTDOMAINDIR);
+ const char *version = glibc_version();
+ int major, minor, vsnprintf_broken;
+
+ if (version sscanf(version, %d.%d, major, minor) == 2
+ (major 2 || (major == 2 minor = 17)))
+ vsnprintf_broken = 0;
+ else
+ vsnprintf_broken = 1;
if (!podir)
podir = GIT_LOCALE_PATH;
bindtextdomain(git, podir);
- setlocale(LC_MESSAGES, );
- init_gettext_charset(git);
+ setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, );
+ init_gettext_charset(git, vsnprintf_broken);
textdomain(git);
}
diff --git a/git-compat-util.h b/git-compat-util.h
index 7776f12..967f452 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -488,11 +488,22 @@ extern int git_vsnprintf(char *str, size_t maxsize,
#ifdef __GLIBC_PREREQ
#if __GLIBC_PREREQ(2, 1)
+#include gnu/libc-version.h
+#define glibc_version() gnu_get_libc_version()
#define HAVE_STRCHRNUL
#define HAVE_MEMPCPY
#endif
#endif
+#ifndef glibc_version
+#ifdef __GNU_LIBRARY__
+#define glibc_version() NULL
+#else
+/* non-glibc platforms, see git_setup_gettext() for 2.17 */
+#define glibc_version() 2.17
+#endif
+#endif
+
#ifndef HAVE_STRCHRNUL
#define strchrnul gitstrchrnul
static inline char *gitstrchrnul(const char *s, int c)
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 12/24] read-cache: read index-v5
On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote: Make git read the index file version 5 without complaining. This version of the reader reads neither the cache-tree nor the resolve undo data, however, it won't choke on an index that includes such data. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Nguyen Thai Ngoc Duy pclo...@gmail.com Helped-by: Thomas Rast tr...@student.ethz.ch Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- [...] +static struct directory_entry *read_directories(unsigned int *dir_offset, + unsigned int *dir_table_offset, + void *mmap, int mmap_size) Minor nit: why is this mmap_size int while all others are unsigned long ? [...] +static int read_entry(struct cache_entry **ce, char *pathname, size_t pathlen, + void *mmap, unsigned long mmap_size, + unsigned int first_entry_offset, + unsigned int foffsetblock) [...] +static int read_entries(struct index_state *istate, struct directory_entry *de, + unsigned int first_entry_offset, void *mmap, + unsigned long mmap_size, unsigned int *nr, + unsigned int foffsetblock) [...] +static int read_index_v5(struct index_state *istate, void *mmap, +unsigned long mmap_size, struct filter_opts *opts) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] replace_object: don't check read_replace_refs twice
Since ecef (inline lookup_replace_object() calls, May 15 2011) the read_replace_refs global variable is checked twice, once in lookup_replace_object() and once again in do_lookup_replace_object(). As do_lookup_replace_object() is called only from lookup_replace_object(), we can remove the check in do_lookup_replace_object(). Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- replace_object.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/replace_object.c b/replace_object.c index d0b1548..cdcaf8c 100644 --- a/replace_object.c +++ b/replace_object.c @@ -97,9 +97,6 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1) int pos, depth = MAXREPLACEDEPTH; const unsigned char *cur = sha1; - if (!read_replace_refs) - return sha1; - prepare_replace_object(); /* Try to recursively replace the object */ -- 1.8.4.1.561.g12affca -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] teach replace objects to sha1_object_info_extended()
Here is a patch series to improve the way sha1_object_info_extended() behaves when it is passed a replaced object. This patch series was inspired by a sub thread in this discussion: http://thread.gmane.org/gmane.comp.version-control.git/238118 Patches 1/5 and 2/5 are cleanups. Patch 4/5 adds a test that fails. Patch 5/5 adds a call to lookup_replace_object_extended() in sha1_object_info_extended() and makes the previous test pass. Christian Couder (5): replace_object: don't check read_replace_refs twice introduce lookup_replace_object_extended() to pass flags Add an unsigned flags parameter to sha1_object_info_extended() t6050: show that git cat-file --batch fails with replace objects sha1_file: perform object replacement in sha1_object_info_extended() builtin/cat-file.c | 2 +- cache.h| 8 +++- replace_object.c | 3 --- sha1_file.c| 20 ++-- streaming.c| 2 +- t/t6050-replace.sh | 5 + 6 files changed, 24 insertions(+), 16 deletions(-) -- 1.8.4.1.561.g12affca -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] introduce lookup_replace_object_extended() to pass flags
Currently, there is only one caller to lookup_replace_object()
that can benefit from passing it some flags, but we expect
that there could be more.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
cache.h | 6 ++
sha1_file.c | 3 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index ce377e1..b845485 100644
--- a/cache.h
+++ b/cache.h
@@ -773,6 +773,12 @@ static inline const unsigned char
*lookup_replace_object(const unsigned char *sh
return sha1;
return do_lookup_replace_object(sha1);
}
+static inline const unsigned char *lookup_replace_object_extended(const
unsigned char *sha1, unsigned flag)
+{
+ if (! (flag READ_SHA1_FILE_REPLACE))
+ return sha1;
+ return lookup_replace_object(sha1);
+}
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/sha1_file.c b/sha1_file.c
index 7dadd04..b0a3964 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2662,8 +2662,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
void *data;
char *path;
const struct packed_git *p;
- const unsigned char *repl = (flag READ_SHA1_FILE_REPLACE)
- ? lookup_replace_object(sha1) : sha1;
+ const unsigned char *repl = lookup_replace_object_extended(sha1, flag);
errno = 0;
data = read_object(repl, type, size);
--
1.8.4.1.561.g12affca
--
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 5/5] sha1_file: perform object replacement in sha1_object_info_extended()
sha1_object_info_extended() should perform object replacement
if it is needed.
The simplest way to do that is to make it call
lookup_replace_object_extended().
And now its unsigned flags parameter is used as it is passed
to lookup_replace_object_extended().
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
sha1_file.c| 13 +++--
t/t6050-replace.sh | 2 +-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 09e56ef..d715553 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2519,8 +2519,9 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi,
struct cached_object *co;
struct pack_entry e;
int rtype;
+ const unsigned char *real = lookup_replace_object_extended(sha1, flags);
- co = find_cached_object(sha1);
+ co = find_cached_object(real);
if (co) {
if (oi-typep)
*(oi-typep) = co-type;
@@ -2532,23 +2533,23 @@ int sha1_object_info_extended(const unsigned char
*sha1, struct object_info *oi,
return 0;
}
- if (!find_pack_entry(sha1, e)) {
+ if (!find_pack_entry(real, e)) {
/* Most likely it's a loose object. */
- if (!sha1_loose_object_info(sha1, oi)) {
+ if (!sha1_loose_object_info(real, oi)) {
oi-whence = OI_LOOSE;
return 0;
}
/* Not a loose object; someone else may have just packed it. */
reprepare_packed_git();
- if (!find_pack_entry(sha1, e))
+ if (!find_pack_entry(real, e))
return -1;
}
rtype = packed_object_info(e.p, e.offset, oi);
if (rtype 0) {
- mark_bad_packed_object(e.p, sha1);
- return sha1_object_info_extended(sha1, oi, 0);
+ mark_bad_packed_object(e.p, real);
+ return sha1_object_info_extended(real, oi, 0);
} else if (in_delta_base_cache(e.p, e.offset)) {
oi-whence = OI_DBCACHED;
} else {
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index b90dbdc..bb785ec 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,7 +276,7 @@ test_expect_success '-f option bypasses the type check' '
git replace -f HEAD^ $BLOB
'
-test_expect_failure 'git cat-file --batch works on replace objects' '
+test_expect_success 'git cat-file --batch works on replace objects' '
git replace | grep $PARA3
echo $PARA3 | git cat-file --batch
'
--
1.8.4.1.561.g12affca
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] t6050: show that git cat-file --batch fails with replace objects
When --batch is passed to git cat-file, the sha1_object_info_extended() function is used to get information about the objects passed to git cat-file. Unfortunately sha1_object_info_extended() doesn't take care of object replacement properly, so it will often fail with a message like this: $ echo a3fb2e1845a1aaf129b7975048973414dc172173 | git cat-file --batch a3fb2e1845a1aaf129b7975048973414dc172173 commit 231 fatal: object a3fb2e1845a1aaf129b7975048973414dc172173 change size!? The goal of this patch is to show this breakage. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 7d47984..b90dbdc 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -276,6 +276,11 @@ test_expect_success '-f option bypasses the type check' ' git replace -f HEAD^ $BLOB ' +test_expect_failure 'git cat-file --batch works on replace objects' ' + git replace | grep $PARA3 + echo $PARA3 | git cat-file --batch +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.8.4.1.561.g12affca -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] Add an unsigned flags parameter to sha1_object_info_extended()
This parameter is not used yet, but it will be used to tell
sha1_object_info_extended() if it should perform object
replacement or not.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/cat-file.c | 2 +-
cache.h| 2 +-
sha1_file.c| 6 +++---
streaming.c| 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..a2d3a9b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -238,7 +238,7 @@ static int batch_one_object(const char *obj_name, struct
batch_options *opt,
return 0;
}
- if (sha1_object_info_extended(data-sha1, data-info) 0) {
+ if (sha1_object_info_extended(data-sha1, data-info,
READ_SHA1_FILE_REPLACE) 0) {
printf(%s missing\n, obj_name);
fflush(stdout);
return 0;
diff --git a/cache.h b/cache.h
index b845485..3be03dc 100644
--- a/cache.h
+++ b/cache.h
@@ -1104,7 +1104,7 @@ struct object_info {
} packed;
} u;
};
-extern int sha1_object_info_extended(const unsigned char *, struct object_info
*);
+extern int sha1_object_info_extended(const unsigned char *, struct object_info
*, unsigned flags);
/* Dumb servers support */
extern int update_server_info(int);
diff --git a/sha1_file.c b/sha1_file.c
index b0a3964..09e56ef 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2514,7 +2514,7 @@ static int sha1_loose_object_info(const unsigned char
*sha1,
return 0;
}
-int sha1_object_info_extended(const unsigned char *sha1, struct object_info
*oi)
+int sha1_object_info_extended(const unsigned char *sha1, struct object_info
*oi, unsigned flags)
{
struct cached_object *co;
struct pack_entry e;
@@ -2548,7 +2548,7 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi)
rtype = packed_object_info(e.p, e.offset, oi);
if (rtype 0) {
mark_bad_packed_object(e.p, sha1);
- return sha1_object_info_extended(sha1, oi);
+ return sha1_object_info_extended(sha1, oi, 0);
} else if (in_delta_base_cache(e.p, e.offset)) {
oi-whence = OI_DBCACHED;
} else {
@@ -2570,7 +2570,7 @@ int sha1_object_info(const unsigned char *sha1, unsigned
long *sizep)
oi.typep = type;
oi.sizep = sizep;
- if (sha1_object_info_extended(sha1, oi) 0)
+ if (sha1_object_info_extended(sha1, oi, READ_SHA1_FILE_REPLACE) 0)
return -1;
return type;
}
diff --git a/streaming.c b/streaming.c
index debe904..9659f18 100644
--- a/streaming.c
+++ b/streaming.c
@@ -113,7 +113,7 @@ static enum input_source istream_source(const unsigned char
*sha1,
oi-typep = type;
oi-sizep = size;
- status = sha1_object_info_extended(sha1, oi);
+ status = sha1_object_info_extended(sha1, oi, 0);
if (status 0)
return stream_error;
--
1.8.4.1.561.g12affca
--
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 12/24] read-cache: read index-v5
On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
+static int verify_hdr(void *mmap, unsigned long size)
+{
+ uint32_t *filecrc;
+ unsigned int header_size;
+ struct cache_header *hdr;
+ struct cache_header_v5 *hdr_v5;
+
+ if (size sizeof(struct cache_header)
+ + sizeof (struct cache_header_v5) + 4)
+ die(index file smaller than expected);
+
+ hdr = mmap;
+ hdr_v5 = ptr_add(mmap, sizeof(*hdr));
+ /* Size of the header + the size of the extensionoffsets */
+ header_size = sizeof(*hdr) + sizeof(*hdr_v5) + hdr_v5-hdr_nextension
* 4;
+ /* Initialize crc */
+ filecrc = ptr_add(mmap, header_size);
+ if (!check_crc32(0, hdr, header_size, ntohl(*filecrc)))
+ return error(bad index file header crc signature);
+ return 0;
+}
I find it curious that we actually need a value from the header (and
use it for pointer arithmetic) to check that the header is valid. The
application will crash before the crc is checked if
hdr_v5-hdr_nextensions is corrupted. Or am I missing something ?
--
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] stash: handle specifying stashes with spaces
Thomas Rast tr at thomasrast.ch writes:
I wonder what we would lose by dropping the --symbolic in the line I
quoted above (which is the second parsing pass), so that it resolves
to a SHA1. We would gain some robustness, as I'm not sure $REV:
works correctly in the face of weird revision expressions like
:/foo.
If I drop --symbolic then all hell breaks loose. Removing it very
naively led to 29 failed tests. I managed to get that down quite a bit
though.
After the function ends $REV is supposed to expand to a symbolic
reference, and the test for whether the given argument is a valid stash
reference is to see if running `git rev-parse --symbolic-full-name
${REV%@*}` expands to 'refs/stash' or not. So we have to do both with
and without --symbolic and keep both around. For example `git stash
drop` had problems because git-reflog doesn't let you remove entries in
the log by SHA1:
$ git reflog delete --updateref --rewrite $(git rev-parse stash@{0})
error: Not a reflog: 418af27beea220ad8a2fd3b8286959b1ec9c8852
I think a not entirely accurate but succinct way of putting it is that
if foo is a valid ref or valid entry in the reflog then
git rev-parse --symbolic $(git rev-parse foo)
does *not* output 'foo' but the SHA1 of 'foo'. So we cannot simply
convert everything to hashes and proceed from there.
Øsse
--
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 09/24] ls-files.c: use index api
On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
@@ -447,6 +463,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
struct dir_struct dir;
struct exclude_list *el;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct filter_opts *opts = xmalloc(sizeof(*opts));
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
N_(paths are separated with NUL character),
@@ -512,9 +529,6 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
prefix_len = strlen(prefix);
git_config(git_default_config, NULL);
- if (read_cache() 0)
- die(index file corrupt);
-
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
el = add_exclude_list(dir, EXC_CMDL, --exclude option);
@@ -550,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
+ if (!with_tree !needs_trailing_slash_stripped()) {
+ memset(opts, 0, sizeof(*opts));
+ opts-pathspec = pathspec;
+ opts-read_staged = 1;
+ if (show_resolve_undo)
+ opts-read_resolve_undo = 1;
+ if (read_cache_filtered(opts) 0)
+ die(index file corrupt);
+ } else {
+ if (read_cache() 0)
+ die(index file corrupt);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
+
+ }
+
Would it make sense to move the declaration of opts as a non-pointer
to the block where it's used ?
--
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
git stash doesn't honor --work-tree or GIT_WORK_TREE
Unlike other commands, git stash doesn't work outside of the worktree, even when --work-tree is specified: abrooks@host:~/tmp$ mkdir test-repo abrooks@host:~/tmp$ cd !$ cd test-repo abrooks@host:~/tmp/test-repo$ git init Initialized empty Git repository in /home/abrooks/tmp/test-repo/.git/ abrooks@host:~/tmp/test-repo$ echo foo foo.txt abrooks@host:~/tmp/test-repo$ git add foo.txt abrooks@host:~/tmp/test-repo$ git commit -m adding foo [master (root-commit) 9d0705e] adding foo 1 file changed, 1 insertion(+) create mode 100644 foo.txt abrooks@host:~/tmp/test-repo$ echo bar foo.txt abrooks@host:~/tmp/test-repo$ cd .. abrooks@host:~/tmp$ git --git-dir=./test-repo/.git --work-tree=./test-repo stash fatal: /usr/lib/git-core/git-stash cannot be used without a working tree. abrooks@host:~/tmp$ cd test-repo/ abrooks@host:~/tmp/test-repo$ git stash Saved working directory and index state WIP on master: 9d0705e adding foo HEAD is now at 9d0705e adding foo abrooks@host:~/tmp/test-repo$ cd ../ abrooks@host:~/tmp$ git --git-dir=./test-repo/.git --work-tree=./test-repo stash list fatal: /usr/lib/git-core/git-stash cannot be used without a working tree. The same also does not work when setting GIT_DIR and GIT_WORK_TREE. As a user, this seems wrong. It looks like the require_work_tree function should check the environment variables in addition to the status of the PWD (via git-rev-parse). Having looked through several of the other git-*.sh scripts, I think other shell based git commands will have similar problems. Thanks, Aaron -- 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 09/24] ls-files.c: use index api
Antoine Pelisse apeli...@gmail.com writes:
On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
@@ -447,6 +463,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
struct dir_struct dir;
struct exclude_list *el;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct filter_opts *opts = xmalloc(sizeof(*opts));
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
N_(paths are separated with NUL character),
@@ -512,9 +529,6 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
prefix_len = strlen(prefix);
git_config(git_default_config, NULL);
- if (read_cache() 0)
- die(index file corrupt);
-
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
el = add_exclude_list(dir, EXC_CMDL, --exclude option);
@@ -550,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const
char *cmd_prefix)
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
+ if (!with_tree !needs_trailing_slash_stripped()) {
+ memset(opts, 0, sizeof(*opts));
+ opts-pathspec = pathspec;
+ opts-read_staged = 1;
+ if (show_resolve_undo)
+ opts-read_resolve_undo = 1;
+ if (read_cache_filtered(opts) 0)
+ die(index file corrupt);
+ } else {
+ if (read_cache() 0)
+ die(index file corrupt);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
+
+ }
+
Would it make sense to move the declaration of opts as a non-pointer
to the block where it's used ?
Yes, I think that would make sense, will do so in the re-roll. 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 v4 12/24] read-cache: read index-v5
Antoine Pelisse apeli...@gmail.com writes: On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote: Make git read the index file version 5 without complaining. This version of the reader reads neither the cache-tree nor the resolve undo data, however, it won't choke on an index that includes such data. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Nguyen Thai Ngoc Duy pclo...@gmail.com Helped-by: Thomas Rast tr...@student.ethz.ch Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- [...] +static struct directory_entry *read_directories(unsigned int *dir_offset, + unsigned int *dir_table_offset, + void *mmap, int mmap_size) Minor nit: why is this mmap_size int while all others are unsigned long ? Thanks for catching that. It should be unsigned long here to. Will fix in the re-roll. [...] +static int read_entry(struct cache_entry **ce, char *pathname, size_t pathlen, + void *mmap, unsigned long mmap_size, + unsigned int first_entry_offset, + unsigned int foffsetblock) [...] +static int read_entries(struct index_state *istate, struct directory_entry *de, + unsigned int first_entry_offset, void *mmap, + unsigned long mmap_size, unsigned int *nr, + unsigned int foffsetblock) [...] +static int read_index_v5(struct index_state *istate, void *mmap, +unsigned long mmap_size, struct filter_opts *opts) -- 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 12/24] read-cache: read index-v5
Antoine Pelisse apeli...@gmail.com writes:
On Wed, Nov 27, 2013 at 1:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
+static int verify_hdr(void *mmap, unsigned long size)
+{
+ uint32_t *filecrc;
+ unsigned int header_size;
+ struct cache_header *hdr;
+ struct cache_header_v5 *hdr_v5;
+
+ if (size sizeof(struct cache_header)
+ + sizeof (struct cache_header_v5) + 4)
+ die(index file smaller than expected);
+
+ hdr = mmap;
+ hdr_v5 = ptr_add(mmap, sizeof(*hdr));
+ /* Size of the header + the size of the extensionoffsets */
+ header_size = sizeof(*hdr) + sizeof(*hdr_v5) +
hdr_v5-hdr_nextension * 4;
+ /* Initialize crc */
+ filecrc = ptr_add(mmap, header_size);
+ if (!check_crc32(0, hdr, header_size, ntohl(*filecrc)))
+ return error(bad index file header crc signature);
+ return 0;
+}
I find it curious that we actually need a value from the header (and
use it for pointer arithmetic) to check that the header is valid. The
application will crash before the crc is checked if
hdr_v5-hdr_nextensions is corrupted. Or am I missing something ?
Good catch, I'm the one that was missing something here. We still need
to use the value from the header before calculating the crc, but should
check if header_size - 4 is less than the total size of the index file.
Then even if the header is corrupted we won't read anything that is not
mmap'ed and thus won't crash.
This guard should also be included for everything else that checks the
crc checksum, as that has the same problems and the calculated place in
the file for the crc might be after the end of the file.
Thanks, will fix in the re-roll.
--
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 stash doesn't honor --work-tree or GIT_WORK_TREE
Aaron Brooks aaron at brooks1.net writes: Unlike other commands, git stash doesn't work outside of the worktree, even when --work-tree is specified: (...) It looks like the require_work_tree function should check the environment variables in addition to the status of the PWD (via git-rev-parse). Having looked through several of the other git-*.sh scripts, I think other shell based git commands will have similar problems. Thanks, Aaron The environment variables are properly exported. I verified this by adding 'echo $GIT_WORK_TREE; echo $GIT_DIR' at the top of git-stash.sh. So these should propagate to child gits just fine, and so it shouldn't be necessary to test them explicitly. The problem seems to be that git rev-parse --is-inside-work-tree does not honor these. In fact it doesn't even honor --git-dir or --work-tree. Judging by the name this may be intentional. In the mean time, if you are able to use Git 1.8.5, `git -C test-repo stash` will work just fine. Øsse -- 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] gettext.c: only work around the vsnprintf bug on glibc 2.17
On 2013-11-30 13.01, Nguyễn Thái Ngọc Duy wrote:
Bug 6530 [1] causes git show v0.99.6~1 to fail with error your
causes or caused (as we have a work around?)
vsnprintf is broken. The workaround avoids that, but it corrupts
system error messages in non-C locales.
[snip]
The bug in glibc has been fixed since 2.17. If git is built with glibc, it can
^^ (Should we name glibc ?)
[snip]
- setlocale(LC_MESSAGES, );
- init_gettext_charset(git);
+ setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, );
1) One thing I don't understand: Why do we need to set LC_ALL ?
The old patch didn't do it, or what do I miss ?
See https://wiki.debian.org/Locale :
Using LC_ALL is strongly discouraged as it overrides everything. Please use it
only when testing and never set it in a startup file.
2) I stole the code partly from here:
http://sourceware.org/bugzilla/show_bug.cgi?id=6530
--
#include stdio.h
#include locale.h
#include gnu/libc-version.h
#define STR ²éľÂíɱ²¡¶¾£¬ÖܺèµtÄúµÄ360²»×¨Òµ£¡
int main(void) {
char buf[200];
setlocale(LC_ALL, );
printf(gnu_glibc_version()=%s\n,
gnu_get_libc_version());
printf(ret(snprintf)=%d\n, snprintf(buf, 150, %.50s, STR));
return 0;
}
--
Then I run it on different machines:
gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */
gnu_glibc_version()=2.11.3 /* Debian Squeze ?*/
gnu_glibc_version()=2.13 /* Debian Wheezy */
ret(snprintf)=50 /* All the 3 above */
-
So could it be that libc is patched in Debian/Ubuntu, and we
can do a runtime check (rather than looking at the version number),
similar to the code above ?
3) The patch didn't break anything here (Debian, Mac OS).
4) Could it be good to have a test case ? Is t0204 good for inspiration ?
5) I can do more testing if needed.
/Torsten
--
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] gettext.c: only work around the vsnprintf bug on glibc 2.17
gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */ Sorry, Copy-paste error: 2.11.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stash: handle specifying stashes with spaces
On Fri, Nov 29, 2013 at 2:22 PM, Øystein Walle oys...@gmail.com wrote:
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index debda7a..0568ec5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -673,4 +673,15 @@ test_expect_success 'store updates stash ref and reflog'
'
grep quux bazzy
'
+test_expect_success 'handle stash specification with spaces' '
+ git stash clear
Do you want at the end of this line?
+ echo pig file
+ git stash
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
+ grep pig file
+'
+
test_done
--
1.8.5.1.g359345f
--
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] gettext.c: only work around the vsnprintf bug on glibc 2.17
On Sun, Dec 1, 2013 at 6:01 AM, Torsten Bögershausen tbo...@web.de wrote:
On 2013-11-30 13.01, Nguyễn Thái Ngọc Duy wrote:
Bug 6530 [1] causes git show v0.99.6~1 to fail with error your
causes or caused (as we have a work around?)
vsnprintf is broken. The workaround avoids that, but it corrupts
system error messages in non-C locales.
[snip]
The bug in glibc has been fixed since 2.17. If git is built with glibc, it
can
^^ (Should we name glibc ?)
No, probably leftover from editing.
[snip]
- setlocale(LC_MESSAGES, );
- init_gettext_charset(git);
+ setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, );
1) One thing I don't understand: Why do we need to set LC_ALL ?
The old patch didn't do it, or what do I miss ?
See https://wiki.debian.org/Locale :
Using LC_ALL is strongly discouraged as it overrides everything. Please use
it only when testing and never set it in a startup file.
I'm fine with changing it back to LC_MESSAGES+LC_TYPE. For the record,
all gtk+ apps do setlocale(LC_ALL, );
2) I stole the code partly from here:
http://sourceware.org/bugzilla/show_bug.cgi?id=6530
--
#include stdio.h
#include locale.h
#include gnu/libc-version.h
#define STR ²éľÂíɱ²¡¶¾£¬ÖܺèµtÄúµÄ360²»×¨Òµ£¡
int main(void) {
char buf[200];
setlocale(LC_ALL, );
printf(gnu_glibc_version()=%s\n,
gnu_get_libc_version());
printf(ret(snprintf)=%d\n, snprintf(buf, 150, %.50s, STR));
return 0;
}
--
Then I run it on different machines:
gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */
gnu_glibc_version()=2.11.3 /* Debian Squeze ?*/
gnu_glibc_version()=2.13 /* Debian Wheezy */
ret(snprintf)=50 /* All the 3 above */
-
So could it be that libc is patched in Debian/Ubuntu, and we
can do a runtime check (rather than looking at the version number),
similar to the code above ?
Good idea. Now I need to install 2.16 for reproducing it :(
3) The patch didn't break anything here (Debian, Mac OS).
4) Could it be good to have a test case ? Is t0204 good for inspiration ?
5) I can do more testing if needed.
/Torsten
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] gettext.c: detect the vsnprintf bug at runtime
Bug 6530 [1] in glibc causes git show v0.99.6~1 to fail with error
your vsnprintf is broken. The workaround avoids that, but it
corrupts system error messages in non-C locales.
The bug has been fixed since 2.17. We could know running glibc version
with gnu_get_libc_version(). But version is not a sure way to detect
the bug because downstream may back port the fix to older versions. Do
a runtime test that immitates the call flow that leads to your
vsnprintf is broken. Only enable the workaround if the test fails.
Tested on Gentoo Linux, glibc 2.16.0 and 2.17, amd64.
[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
v3 goes with runtime test instead of version check.
gettext.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/gettext.c b/gettext.c
index 71e9545..eed7c7f 100644
--- a/gettext.c
+++ b/gettext.c
@@ -29,6 +29,17 @@ int use_gettext_poison(void)
#endif
#ifndef NO_GETTEXT
+static int test_vsnprintf(const char *fmt, ...)
+{
+ char buf[26];
+ int ret;
+ va_list ap;
+ va_start(ap, fmt);
+ ret = vsnprintf(buf, sizeof(buf), fmt, ap);
+ va_end(ap);
+ return ret;
+}
+
static const char *charset;
static void init_gettext_charset(const char *domain)
{
@@ -99,9 +110,7 @@ static void init_gettext_charset(const char *domain)
$ LANGUAGE= LANG=de_DE.utf8 ./test
test: Kein passendes Ger?t gefunden
- In the long term we should probably see about getting that
- vsnprintf bug in glibc fixed, and audit our code so it won't
- fall apart under a non-C locale.
+ The vsnprintf bug has been fixed since glibc 2.17.
Then we could simply set LC_CTYPE from the environment, which would
make things like the external perror(3) messages work.
@@ -115,7 +124,9 @@ static void init_gettext_charset(const char *domain)
setlocale(LC_CTYPE, );
charset = locale_charset();
bind_textdomain_codeset(domain, charset);
- setlocale(LC_CTYPE, C);
+ /* the string is taken from v0.99.6~1 */
+ if (test_vsnprintf(%.*s, 13, David_K\345gedal) 0)
+ setlocale(LC_CTYPE, C);
}
void git_setup_gettext(void)
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
GIT_DIR not auto ignored
Greetings, I found this probable bug: https://gist.github.com/anonymous/01979fd9e6e285df41a2 Cheers, Ingy döt 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
[PATCH 2/5] builtin/remote: remove postfixcmp() and use suffixcmp() instead
Commit 8cc5b290 (git merge -Xoption, 25 Nov 2009) introduced
suffixcmp() with nearly the same implementation as postfixcmp()
that already existed since commit 211c8968 (Make git-remote a
builtin, 29 Feb 2008).
The only difference between the two implementations is that,
when the string is smaller than the suffix, one implementation
returns 1 while the other one returns -1.
But, as postfixcmp() is only used to compare for equality, the
distinction does not matter and does not affect the correctness of
this patch.
As postfixcmp() has always been static in builtin/remote.c
and is used nowhere else, it makes more sense to remove it
and use suffixcmp() instead in builtin/remote.c, rather than
to remove suffixcmp().
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/remote.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 4e14891..9b3a98e 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -80,14 +80,6 @@ static int verbose;
static int show_all(void);
static int prune_remote(const char *remote, int dry_run);
-static inline int postfixcmp(const char *string, const char *postfix)
-{
- int len1 = strlen(string), len2 = strlen(postfix);
- if (len1 len2)
- return 1;
- return strcmp(string + len1 - len2, postfix);
-}
-
static int fetch_remote(const char *name)
{
const char *argv[] = { fetch, name, NULL, NULL };
@@ -277,13 +269,13 @@ static int config_read_branches(const char *key, const
char *value, void *cb)
enum { REMOTE, MERGE, REBASE } type;
key += 7;
- if (!postfixcmp(key, .remote)) {
+ if (!suffixcmp(key, .remote)) {
name = xstrndup(key, strlen(key) - 7);
type = REMOTE;
- } else if (!postfixcmp(key, .merge)) {
+ } else if (!suffixcmp(key, .merge)) {
name = xstrndup(key, strlen(key) - 6);
type = MERGE;
- } else if (!postfixcmp(key, .rebase)) {
+ } else if (!suffixcmp(key, .rebase)) {
name = xstrndup(key, strlen(key) - 7);
type = REBASE;
} else
--
1.8.4.1.561.g12affca
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] strbuf: introduce starts_with() and ends_with()
prefixcmp() and suffixcmp() cannot be really used as comparison
functions as they are not antisymmetric:
prefixcmp(foo, foobar) 0
prefixcmp(foobar, foo) == 0
So they are not suitable as functions for passing to qsort.
And in fact they are used nowhere as comparison functions.
Therefore we should replace them with functions that just check
for equality.
As a first step toward this goal, this patch introduces
starts_with() and end_with() that will be used to replace
respectively prefixcmp() and suffixcmp().
Some popular programming languages, like Java, Python and Ruby
have functions or methods called like starts_with() and
ends_with() that are doing what we want. Therefore it makes sense
to use such names.
In vcs-svn/fast_export.c, there was already an ends_with()
function that did the same thing. Let's use the new one instead
while at it.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
git-compat-util.h | 2 ++
strbuf.c | 18 ++
vcs-svn/fast_export.c | 11 +--
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 7776f12..b73916b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -350,7 +350,9 @@ extern void set_die_routine(NORETURN_PTR void
(*routine)(const char *err, va_lis
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
extern void set_die_is_recursing_routine(int (*routine)(void));
+extern int starts_with(const char *str, const char *prefix);
extern int prefixcmp(const char *str, const char *prefix);
+extern int ends_with(const char *str, const char *suffix);
extern int suffixcmp(const char *str, const char *suffix);
static inline const char *skip_prefix(const char *str, const char *prefix)
diff --git a/strbuf.c b/strbuf.c
index 1170d01..83caf4a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,6 +1,15 @@
#include cache.h
#include refs.h
+int starts_with(const char *str, const char *prefix)
+{
+ for (; ; str++, prefix++)
+ if (!*prefix)
+ return 1;
+ else if (*str != *prefix)
+ return 0;
+}
+
int prefixcmp(const char *str, const char *prefix)
{
for (; ; str++, prefix++)
@@ -10,6 +19,15 @@ int prefixcmp(const char *str, const char *prefix)
return (unsigned char)*prefix - (unsigned char)*str;
}
+int ends_with(const char *str, const char *suffix)
+{
+ int len = strlen(str), suflen = strlen(suffix);
+ if (len suflen)
+ return 0;
+ else
+ return !strcmp(str + len - suflen, suffix);
+}
+
int suffixcmp(const char *str, const char *suffix)
{
int len = strlen(str), suflen = strlen(suffix);
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index f2b23c8..bd0f2c2 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -162,22 +162,13 @@ static void die_short_read(struct line_buffer *input)
die(invalid dump: unexpected end of file);
}
-static int ends_with(const char *s, size_t len, const char *suffix)
-{
- const size_t suffixlen = strlen(suffix);
- if (len suffixlen)
- return 0;
- return !memcmp(s + len - suffixlen, suffix, suffixlen);
-}
-
static int parse_cat_response_line(const char *header, off_t *len)
{
- size_t headerlen = strlen(header);
uintmax_t n;
const char *type;
const char *end;
- if (ends_with(header, headerlen, missing))
+ if (ends_with(header, missing))
return error(cat-blob reports missing blob: %s, header);
type = strstr(header, blob );
if (!type)
--
1.8.4.1.561.g12affca
--
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 5/5] strbuf: remove prefixcmp() and suffixcmp()
As starts_with() and ends_with() have been used to
replace prefixcmp() and suffixcmp() respectively,
we can now remove them.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
git-compat-util.h | 2 --
strbuf.c | 18 --
2 files changed, 20 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index b73916b..c4c01e7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -351,9 +351,7 @@ extern void set_error_routine(void (*routine)(const char
*err, va_list params));
extern void set_die_is_recursing_routine(int (*routine)(void));
extern int starts_with(const char *str, const char *prefix);
-extern int prefixcmp(const char *str, const char *prefix);
extern int ends_with(const char *str, const char *suffix);
-extern int suffixcmp(const char *str, const char *suffix);
static inline const char *skip_prefix(const char *str, const char *prefix)
{
diff --git a/strbuf.c b/strbuf.c
index 83caf4a..ee96dcf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -10,15 +10,6 @@ int starts_with(const char *str, const char *prefix)
return 0;
}
-int prefixcmp(const char *str, const char *prefix)
-{
- for (; ; str++, prefix++)
- if (!*prefix)
- return 0;
- else if (*str != *prefix)
- return (unsigned char)*prefix - (unsigned char)*str;
-}
-
int ends_with(const char *str, const char *suffix)
{
int len = strlen(str), suflen = strlen(suffix);
@@ -28,15 +19,6 @@ int ends_with(const char *str, const char *suffix)
return !strcmp(str + len - suflen, suffix);
}
-int suffixcmp(const char *str, const char *suffix)
-{
- int len = strlen(str), suflen = strlen(suffix);
- if (len suflen)
- return -1;
- else
- return strcmp(str + len - suflen, suffix);
-}
-
/*
* Used as the default -buf value, so that people can always assume
* buf is non NULL and -buf is NUL terminated even for a freshly
--
1.8.4.1.561.g12affca
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] use starts_with() and ends_with()
This is a new patch series along the lines Junio suggested in this
thread:
http://thread.gmane.org/gmane.comp.version-control.git/238054/
I send it now because I saw a 1.8.5 tag.
The patches in this series can be related to what Junio suggested
this way:
* A set of clean-up patches to normalize oddball usages of existing
functions (e.g. normalize 'prefixcmp(a,b) != 0' in some file(s)
to 'prefixcmp(a,b)');
- Patches 1/5 and 2/5 are such kind of cleanups.
* A single patch to introduce the new function(s), to be applied on
top of 1.8.5;
- Patch 3/5 does that.
* A large patch to convert all uses of prefixcmp to starts_with and
suffixcmp to ends_with in the 1.8.5 codebase;
- Patch 4/5 does that.
* A patch for each topic in flight to convert newly introduced
prefixcmp/suffixcmp to starts_with/ends_with, to be applied after
the topic graduates to 'master' after 1.8.5; and then finally
- I didn't start to work on that yet. I hope that I will only need
to take care of what is going on in 'next'.
* A separate patch to remove prefixcmp and suffixcmp, to be applied
after _all_ in-flight topic has graduated to 'master'.
- Patch 5/5 does that.
Christian Couder (5):
environment: normalize use of prefixcmp() by removing != 0
builtin/remote: remove postfixcmp() and use suffixcmp() instead
strbuf: introduce starts_with() and ends_with()
Replace {pre,suf}fixcmp() with {starts,ends}_with()
strbuf: remove prefixcmp() and suffixcmp()
alias.c | 2 +-
attr.c| 2 +-
bisect.c | 4 +--
branch.c | 4 +--
builtin/apply.c | 12 +++
builtin/archive.c | 4 +--
builtin/branch.c | 6 ++--
builtin/checkout.c| 8 ++---
builtin/clean.c | 4 +--
builtin/clone.c | 8 ++---
builtin/column.c | 2 +-
builtin/commit.c | 10 +++---
builtin/describe.c| 2 +-
builtin/fast-export.c | 2 +-
builtin/fetch-pack.c | 6 ++--
builtin/fetch.c | 18 +--
builtin/fmt-merge-msg.c | 10 +++---
builtin/for-each-ref.c| 14 -
builtin/fsck.c| 6 ++--
builtin/help.c| 8 ++---
builtin/index-pack.c | 8 ++---
builtin/init-db.c | 2 +-
builtin/log.c | 8 ++---
builtin/ls-remote.c | 4 +--
builtin/mailinfo.c| 16 +-
builtin/merge-recursive.c | 4 +--
builtin/merge.c | 12 +++
builtin/name-rev.c| 6 ++--
builtin/notes.c | 2 +-
builtin/pack-objects.c| 2 +-
builtin/prune.c | 4 +--
builtin/receive-pack.c| 6 ++--
builtin/reflog.c | 4 +--
builtin/remote.c | 22 +
builtin/repack.c | 2 +-
builtin/rev-parse.c | 24 +++---
builtin/send-pack.c | 8 ++---
builtin/shortlog.c| 6 ++--
builtin/show-branch.c | 20 ++--
builtin/show-ref.c| 6 ++--
builtin/symbolic-ref.c| 2 +-
builtin/tag.c | 2 +-
builtin/tar-tree.c| 2 +-
builtin/unpack-objects.c | 2 +-
builtin/update-ref.c | 10 +++---
builtin/upload-archive.c | 2 +-
commit.c | 6 ++--
config.c | 16 +-
connect.c | 2 +-
connected.c | 2 +-
convert.c | 2 +-
daemon.c | 40
diff.c| 56 -
environment.c | 2 +-
fast-import.c | 80 +++
fetch-pack.c | 12 +++
git-compat-util.h | 4 +--
git.c | 12 +++
help.c| 8 ++---
http-backend.c| 4 +--
http-push.c | 4 +--
http.c| 10 +++---
imap-send.c | 10 +++---
log-tree.c| 8 ++---
merge-recursive.c | 6 ++--
notes-utils.c | 4 +--
notes.c | 8 ++---
pager.c | 2 +-
parse-options.c | 12 +++
pathspec.c| 2 +-
pkt-line.c| 4 +--
pretty.c | 36 ++---
refs.c| 30 +-
remote-curl.c | 14 -
remote-testsvn.c | 10 +++---
remote.c | 46 +--
revision.c| 38 +++---
send-pack.c | 4 +--
sequencer.c | 8 ++---
setup.c | 4 +--
sha1_name.c | 16 +-
shell.c | 2 +-
strbuf.c | 12 +++
submodule.c | 2 +-
tag.c | 10 +++---
test-line-buffer.c| 6 ++--
test-string-list.c| 2 +-
transport-helper.c| 16 +-
transport.c | 28
[PATCH 1/5] environment: normalize use of prefixcmp() by removing != 0
To be able to automatically convert prefixcmp() to starts_with()
we need first to make sure that prefixcmp() is always used in
the same way.
So let's remove != 0 after prefixcmp().
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
environment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index 0a15349..cd2b068 100644
--- a/environment.c
+++ b/environment.c
@@ -171,7 +171,7 @@ const char *get_git_namespace(void)
const char *strip_namespace(const char *namespaced_ref)
{
- if (prefixcmp(namespaced_ref, get_git_namespace()) != 0)
+ if (prefixcmp(namespaced_ref, get_git_namespace()))
return NULL;
return namespaced_ref + namespace_len;
}
--
1.8.4.1.561.g12affca
--
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
