[PATCH] git.c: add --index-file command-line option.
Unlike other environment variables (e.g. GIT_WORK_TREE,
GIT_NAMESPACE), it was not possible to set the GIT_INDEX_FILE
environment variable using the command line.
Add a new --index-file command-line option.
Update the t7500-commit test to include --index-file option coverage.
The tests have been adapted from the existing
'using alternate GIT_INDEX_FILE (1)' and
'using alternate GIT_INDEX_FILE (2)' tests.
Signed-off-by: Manlio Perillo manlio.peri...@gmail.com
---
Documentation/git.txt | 10 +-
git.c | 17 -
t/t7500-commit.sh | 29 +
3 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index e643683..5a582dd 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -12,7 +12,8 @@ SYNOPSIS
'git' [--version] [--help] [-c name=value]
[--exec-path[=path]] [--html-path] [--man-path] [--info-path]
[-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
-[--git-dir=path] [--work-tree=path] [--namespace=name]
+[--git-dir=path] [--work-tree=path] [--index-file=path]
+[--namespace=name]
command [args]
DESCRIPTION
@@ -408,6 +409,12 @@ help ...`.
variable (see core.worktree in linkgit:git-config[1] for a
more detailed discussion).
+--index-file=path::
+ Set the path to the index file. It can be an absolute path
+ or a path relative to the current working directory.
+ This can also be controlled by setting the GIT_INDEX_FILE
+ environment variable.
+
--namespace=path::
Set the git namespace. See linkgit:gitnamespaces[7] for more
details. Equivalent to setting the `GIT_NAMESPACE` environment
@@ -632,6 +639,7 @@ git so take care if using Cogito etc.
This environment allows the specification of an alternate
index file. If not specified, the default of `$GIT_DIR/index`
is used.
+ The '--index-file command-line option also sets this value.
'GIT_OBJECT_DIRECTORY'::
If the object storage directory is specified via this
diff --git a/git.c b/git.c
index d33f9b3..b0f473d 100644
--- a/git.c
+++ b/git.c
@@ -8,7 +8,8 @@
const char git_usage_string[] =
git [--version] [--exec-path[=path]] [--html-path] [--man-path]
[--info-path]\n
[-p|--paginate|--no-pager] [--no-replace-objects]
[--bare]\n
- [--git-dir=path] [--work-tree=path]
[--namespace=name]\n
+ [--git-dir=path] [--work-tree=path]
[--index-file=path]\n
+ [--namespace=name]\n
[-c name=value] [--help]\n
command [args];
@@ -121,6 +122,20 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strcmp(cmd, --index-file)) {
+ if (*argc 2) {
+ fprintf(stderr, No path given for
--index-file.\n );
+ usage(git_usage_string);
+ }
+ setenv(INDEX_ENVIRONMENT, (*argv)[1], 1);
+ if (envchanged)
+ *envchanged = 1;
+ (*argv)++;
+ (*argc)--;
+ } else if (!prefixcmp(cmd, --index-file=)) {
+ setenv(INDEX_ENVIRONMENT, cmd + 13, 1);
+ if (envchanged)
+ *envchanged = 1;
} else if (!strcmp(cmd, --bare)) {
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 1c908f4..c405a78 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -168,6 +168,35 @@ test_expect_success 'using alternate GIT_INDEX_FILE (2)' '
cmp .git/index saved-index /dev/null
'
+test_expect_success 'using alternate --index-file (1)' '
+
+ cp .git/index saved-index
+ (
+ echo some new content file
+ index_file=.git/another_index
+ git --index-file=$index_file add file
+ git --index-file=$index_file commit -m commit using another
index
+ git --index-file=$index_file diff-index --exit-code HEAD
+ git --index-file=$index_file diff-files --exit-code
+ )
+ cmp .git/index saved-index /dev/null
+
+'
+
+test_expect_success 'using alternate --index-file (2)' '
+
+ cp .git/index saved-index
+ (
+ rm -f .git/no-such-index
+ index_file=.git/no-such-index
+ git --index-file=$index_file commit -m commit using
nonexistent index
+ test -z $(git --index-file=$index_file ls-files)
+ test -z $(git
Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
Felipe, please stop referring to facts and obvious. You pretend to be a being of pure reason and that everything you say is logical, drawn from facts. But you forget or perhaps do not know that logic by itself proofs nothing, it all depends on the axioms you impose. And yours are quite different from what others consider as such, and apparently also inconsistent. So, instead of trying to twist things around so that broken things in your code are not broken after all, why not simply re-roll your patch with the obvious fixes applies? As you write yourself, time is not pressing at all -- so I don't see why your patch should be merged now, and fixed later, contrary to how other people's patches are treated? Why not fix them first, and then apply? We do have time, after all! And nobody is expecting you to do that while you are on vacation, either. Nor that you do it instantly. Just say: OK, I see there is a problem with the patches; even though I consider it unimportant, I will play by the rules everybody here has to follow, and re-roll the patch series. But this is of low priority for me, so I cannot say right now when it will happen. Everybody would be happy then. Except perhaps the hypothetical users, who would have to wait a bit longer -- but oh, not really, because they can just use remote-bzr from your repo, yay :-). I really like that about it, it lives in contrib, so one can use it w/o it being merged into git.git. Instead, you make claims that make you look like a foolish and arrogant ass, all the while insulting Junio and me implicitly. Why do you do that??? It delays acceptance of your nice work. As you write, this hurts the users. So why do it? Since you keep complaining that nobody ever really can point to anything wrong your said, I'll do you the favor by deconstructing one of the claims you made: On 13.12.2012, at 20:06, Felipe Contreras wrote: On Thu, Dec 13, 2012 at 6:04 AM, Max Horn post...@quendi.de wrote: On 13.12.2012, at 11:08, Felipe Contreras wrote: On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: New remote helper for bzr (v3). With minor fixes, this may be ready for 'next'. What minor fixes? Lookng at the above (fixup), $gmane/210744 comes to mind That doesn't matter. The code and the tests would work just fine. It doesn't matter? I find that statement hard to align with what the maintainer of git, and thus the person who decides whether your patch series gets merged or not, wrote just above? In fact, it seems to me that what Junio said matters a great deal... So you think Junio knows more about remote-bzr than I do? This is a classical straw man argument. No, I do not think that. But I do think that Junio knows enough to review your code, and I do think that the point he raised is valid. You disagree with the importance of his point I repeat; it doesn't affect the tests, it doesn't affect the code, it doesn't cause any problem. remote-bzr could be merged today, in fact, it could have been merged a month ago. You don't trust me? Here, look: [..] All this code is a no-op, because, as Junio pointed out, cmd is null. How is that a problem? It's not. It is a problem. Because either the code inside the if is important, and then this is a bug. Or it is not important -- then it should not be there in the first place. Either way, the patch series should be re-rolled. Of course in a whatever time frame suits you. If you are not willing to do that, this is sad, but of course also your right! [...] This is a very strange attitude... In another email, you complained about nobody reviewing your patches respectively nobody voicing any constructive criticism. Yet Junio did just that, and again in $gmane/210745 -- and you replied to neither, and acted on neither (not even by refuting the points brought up), and now summarily dismiss them as irrelevant. I find that quite disturbing :-(. I didn't say it was irrelevant, it should be fixed, Actually, you wrote: That doesn't matter. So I paraphrased. In any case, I am glad to hear you finally agree that it should be fixed (which you did *not* say in your initial reply). So the problem we have seems to be that you do not understand how patches typically handled in git.git. Well, based on my observation: If reviews point out things in a patch series that are not optimal or even broken, it is expected that the submitter fixes this locally and resubmits a new version of the series. In some cases, it is possible to make exceptions, e.g. trivial typo fixes can be applied on the fly. But otherwise, you re-roll, you do not get your stuff merged just based on the promise that you'll submit a series of fixes later. Esp. if the fixes are relatively easy. Of course more exceptions can be made, but based on what I saw, this is rare, and has to be justified quite well. I
Re: How to avoid the ^M induced by Meld and Git
FYI: It was all down to unexpected dos formatting of one of the files. Here's how i sorted out my unwanted ^M line endings in case anyone stumbles on this thread with the same issue i had. (reposted from http://stackoverflow.com/questions/13799631/why-is-m-being-added-to-a-script-r-after-modifying-with-meld/13879162#13879162 ) Thanks again to those investing effort on this issue - cheers! In a terminal i ran cat script.r | hexdump -C | head and found a 0d 0a in the file, which is DOS formatting for a new line (carriage return 0d immediately followed by a line feed 0a). I ran the same command on another_script.r i was merging with but only observed 0a, no 0d 0a, indicating Unix formatting. To check further if this was the source of the ^M line endings, script.r was converted to unix formatting via dos2unix script.r verified that 0d 0a was converted to 0a using hexdump -C as above. I performed a merge using Meld in attempting to replicate the process which yielded ^M line endings in my script's. I re-oppened both files in Emacs/ESS and found no ^M line endings. Short of converting script.r back to dos formatting and repeating the above procedure to see if the ^M line endings re-appear, i believe i've solved my ^M issue, which simply is that, unbeknownst to me, one of my files was dos formatted. My take home message: in a Windows dominated environ, never assume that one's personal linux environment doesn't contain DOS bits. Or line endings. On 12/12/12 17:17, Karl Brand wrote: On 12/12/12 17:08, Michael J Gruber wrote: Karl Brand venit, vidit, dixit 12.12.2012 16:34: On 12/12/12 15:57, Michael J Gruber wrote: Karl Brand venit, vidit, dixit 11.12.2012 13:33: Esteemed Git users, What i do: 1. Create a script.r using Emacs/ESS. 2. Make some modifications to script.r with the nice diff gui, Meld 3. Commit these modifications using git commit -am my message 4. Reopen script.r in Emacs/ESS to continue working. The lines added (/edited ?) using Meld all end with ^M which i certainly don't want. Lines not added/edited with Meld do NOT end with ^M. What happens if you leave out step 3? If the same happens then Meld is the culprit. (Unless you've set some special options, git does not modify your file on commit, so this can't be git related.) Leaving out step 3. results in exactly the same thing. Thus Git doesn't appear to be responsible for the ^M's. Thanks a lot for your effort on this and my apologies for not taking the care to dissect this more carefully as you suggested. Over to the Meld mailing list... I didn't mean to shy you away ;) Applying recent lessons form StackO'flow :P Could it be that there is a ^M very early in the file (or rather something else which isn't covered by dos2unix) so that Meld thinks it's DOS and inserts line endings as DOS? At least that's what vim would do. If there is i don't find it using Emacs, but Emacs may only show what dos2unix sees... Will post back the Meld insights (here's hoping!) In any case, Meld people over there should know (or get to know) that effect. There are plenty of posts around about these being line endings used for windows which can appear when working on a script under a *nix OS which has previously been edited in a Windows OS. This is not the case here - everything is taking place on Ubuntu 12.04. FWIW: the directory is being synced by dropbox; and in Meld, Preferences Encoding tab, utf8 is entered in the text box. Current work around is running in a terminal: dos2unix /path/to/script.r which strips the ^M's But this just shouldn't be necessary and I'd really appreciate the reflections advice on how to stop inducing these ^M's ! With thanks, Karl (re)posted here as suggested off topic at SO: http://stackoverflow.com/questions/13799631/create-script-r-in-emacs-modify-with-meld-git-commit-reopen-in-emacs-m -- Karl Brand Dept of Cardiology and Dept of Bioinformatics Erasmus MC Dr Molewaterplein 50 3015 GE Rotterdam T +31 (0)10 703 2460 |M +31 (0)642 777 268 |F +31 (0)10 704 4161 -- 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] Documentation/git: add missing info about --git-dir command-line option
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 12/12/2012 20:35, Junio C Hamano ha scritto: Manlio Perillo manlio.peri...@gmail.com writes: The Documentation/git.txt file, in the GIT_DIR environment variable section, did not mentioned that this value can also be set using the --git-dir command line option. --- s/mentioned/mention/; Also it may help to say Unlike other environment variables (e.g. GIT_WORK_TREE, GIT_NAMESPACE), I'm sorry, I just copied this text as is (I'm lazy) in the commit message failing to notice the use of the tab character. When I checked the patch email message, my editor rendered the tab character as a single space... That's the reason why I have all my editors configured to never ever use tabs. Manlio Perillo -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDLYUMACgkQscQJ24LbaUTUPACcDhufXkawZZPBV0p/af1GFu1D /BcAnjPARpeTi4EdyM/3wV0eI9U9Fu51 =rSfl -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Port to QNX
From: Matt Kraai matt.kr...@amo.abbott.com Signed-off-by: Matt Kraai matt.kr...@amo.abbott.com --- Makefile | 19 +++ git-compat-util.h | 8 ++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 736ecd4..ed2539d 100644 --- a/Makefile +++ b/Makefile @@ -78,6 +78,8 @@ all:: # # Define NO_MEMMEM if you don't have memmem. # +# Define NO_GETPAGESIZE if you don't have getpagesize. +# # Define NO_STRLCPY if you don't have strlcpy. # # Define NO_STRTOUMAX if you don't have both strtoimax and strtoumax in the @@ -1448,6 +1450,20 @@ else NO_CURL = YesPlease endif endif +ifeq ($(uname_S),QNX) + COMPAT_CFLAGS += -DSA_RESTART=0 + NEEDS_SOCKET = YesPlease + NO_MKDTEMP = YesPlease + NO_MKSTEMPS = YesPlease + NO_FNMATCH_CASEFOLD = YesPlease + NO_GETPAGESIZE = YesPlease + NO_ICONV = YesPlease + NO_MEMMEM = YesPlease + NO_NSEC = YesPlease + NO_STRCASESTR = YesPlease + NO_STRLCPY = YesPlease + PTHREAD_LIBS = +endif -include config.mak.autogen -include config.mak @@ -1859,6 +1875,9 @@ ifdef NO_MEMMEM COMPAT_CFLAGS += -DNO_MEMMEM COMPAT_OBJS += compat/memmem.o endif +ifdef NO_GETPAGESIZE + COMPAT_CFLAGS += -DNO_GETPAGESIZE +endif ifdef INTERNAL_QSORT COMPAT_CFLAGS += -DINTERNAL_QSORT COMPAT_OBJS += compat/qsort.o diff --git a/git-compat-util.h b/git-compat-util.h index 2e79b8a..6c588ca 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -75,7 +75,7 @@ # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ - !defined(__TANDEM) + !defined(__TANDEM) !defined(__QNX__) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */ #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif @@ -99,7 +99,7 @@ #include stdlib.h #include stdarg.h #include string.h -#ifdef __TANDEM /* or HAVE_STRINGS_H or !NO_STRINGS_H? */ +#if defined(__TANDEM) || defined(__QNX__) /* or HAVE_STRINGS_H or !NO_STRINGS_H? */ #include strings.h /* for strcasecmp() */ #endif #include errno.h @@ -411,6 +411,10 @@ void *gitmemmem(const void *haystack, size_t haystacklen, const void *needle, size_t needlelen); #endif +#ifdef NO_GETPAGESIZE +#define getpagesize() sysconf(_SC_PAGESIZE) +#endif + #ifdef FREAD_READS_DIRECTORIES #ifdef fopen #undef fopen -- 1.8.1-rc1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Port to QNX
This series ports Git to QNX. It builds on both QNX 6.3.2 and QNX 6.5.0. The test suite does not pass. Unless the corresponding software is installed, the following arguments must be passed to Make: NO_CURL=1 NO_GETTEXT=1 NO_OPENSSL=1 NO_PERL=1 NO_PYTHON=1 NO_TCLTK=1 [1/2]: Make lock local to fetch_pack QNX 6.3.2's unistd.h declares a function named lock, which causes fetch-pack.c to fail to compile if lock has file-scope. Since it's only used in a single function, move it therein. [2/2]: Port to QNX -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Make lock local to fetch_pack
From: Matt Kraai matt.kr...@amo.abbott.com
lock is only used by fetch_pack, so move it into that function.
Signed-off-by: Matt Kraai matt.kr...@amo.abbott.com
---
fetch-pack.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 099ff4d..9d9762d 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -874,8 +874,6 @@ static int fetch_pack_config(const char *var, const char
*value, void *cb)
return git_default_config(var, value, cb);
}
-static struct lock_file lock;
-
static void fetch_pack_setup(void)
{
static int did_setup;
@@ -896,6 +894,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
struct string_list *sought,
char **pack_lockfile)
{
+ static struct lock_file lock;
struct stat st;
struct ref *ref_cpy;
--
1.8.1-rc1
--
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: Build fixes for another obscure Unix
Hi, On Fri, Dec 14, 2012 at 2:54 AM, Joachim Schmitz j...@schmitz-digital.de wrote: For what's it worth: I ACK your HP-NonStop patch (as you can see by my comment in git-compat-util.h I was thinking along the same line) https://github.com/dm0-/git/commit/933d72a5cfdc63fa9c3c68afa2f4899d9c3f791e together with its prerequisit https://github.com/dm0-/git/commit/301032c6488aeabb94ccc81bfb6d65ff2c23b924 ACKed by: Joachim Schmitz j...@schmitz-digital.de Okay, thanks for verifying. Especially since another port needing that header was just sent to the list, I'd prefer to see some generalized feature test rather than building and maintaining an explicit OS list. No one has suggested any adjustments, so I'll send out those patches now. Thanks. David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Support builds when sys/param.h is missing
An option is added to the Makefile to skip the inclusion of sys/param.h. The only known platform with this condition thus far is the z/OS UNIX System Services environment. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 5 + configure.ac | 6 ++ git-compat-util.h | 2 ++ 3 files changed, 13 insertions(+) diff --git a/Makefile b/Makefile index 736ecd4..8c3a0dd 100644 --- a/Makefile +++ b/Makefile @@ -165,6 +165,8 @@ all:: # Define NO_POLL if you do not have or don't want to use poll(). # This also implies NO_SYS_POLL_H. # +# Define NO_SYS_PARAM_H if you don't have sys/param.h. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1748,6 +1750,9 @@ endif ifdef NO_SYS_POLL_H BASIC_CFLAGS += -DNO_SYS_POLL_H endif +ifdef NO_SYS_PARAM_H +BASIC_CFLAGS += -DNO_SYS_PARAM_H +endif ifdef NO_INTTYPES_H BASIC_CFLAGS += -DNO_INTTYPES_H endif diff --git a/configure.ac b/configure.ac index ad215cc..317bfc6 100644 --- a/configure.ac +++ b/configure.ac @@ -699,6 +699,12 @@ AC_CHECK_HEADER([sys/poll.h], [NO_SYS_POLL_H=UnfortunatelyYes]) GIT_CONF_SUBST([NO_SYS_POLL_H]) # +# Define NO_SYS_PARAM_H if you don't have sys/param.h +AC_CHECK_HEADER([sys/param.h], +[NO_SYS_PARAM_H=], +[NO_SYS_PARAM_H=UnfortunatelyYes]) +GIT_CONF_SUBST([NO_SYS_PARAM_H]) +# # Define NO_INTTYPES_H if you don't have inttypes.h AC_CHECK_HEADER([inttypes.h], [NO_INTTYPES_H=], diff --git a/git-compat-util.h b/git-compat-util.h index 2e79b8a..ace1549 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -104,7 +104,9 @@ #endif #include errno.h #include limits.h +#ifndef NO_SYS_PARAM_H #include sys/param.h +#endif #include sys/types.h #include dirent.h #include sys/time.h -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] Detect when the passwd struct is missing pw_gecos
NO_GECOS_IN_PWENT was documented with other Makefile variables but was only enforced by manually defining it to the C preprocessor. This adds support for detecting the condition with configure and defining the make variable. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 3 +++ configure.ac | 8 2 files changed, 11 insertions(+) diff --git a/Makefile b/Makefile index 8c3a0dd..735a834 100644 --- a/Makefile +++ b/Makefile @@ -1657,6 +1657,9 @@ endif ifdef NO_D_INO_IN_DIRENT BASIC_CFLAGS += -DNO_D_INO_IN_DIRENT endif +ifdef NO_GECOS_IN_PWENT +BASIC_CFLAGS += -DNO_GECOS_IN_PWENT +endif ifdef NO_ST_BLOCKS_IN_STRUCT_STAT BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT endif diff --git a/configure.ac b/configure.ac index 317bfc6..3347c17 100644 --- a/configure.ac +++ b/configure.ac @@ -759,6 +759,14 @@ AC_CHECK_MEMBER(struct dirent.d_type, [#include dirent.h]) GIT_CONF_SUBST([NO_D_TYPE_IN_DIRENT]) # +# Define NO_GECOS_IN_PWENT if you don't have pw_gecos in struct passwd +# in the C library. +AC_CHECK_MEMBER(struct passwd.pw_gecos, +[NO_GECOS_IN_PWENT=], +[NO_GECOS_IN_PWENT=YesPlease], +[#include pwd.h]) +GIT_CONF_SUBST([NO_GECOS_IN_PWENT]) +# # Define NO_SOCKADDR_STORAGE if your platform does not have struct # sockaddr_storage. AC_CHECK_TYPE(struct sockaddr_storage, -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] Declare that HP NonStop systems require strings.h
This platform previously included strings.h automatically. However, the build system now requires an explicit option to do so. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fb78f7f..e84b0cb 100644 --- a/Makefile +++ b/Makefile @@ -1357,6 +1357,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # Added manually, see above. NEEDS_SSL_WITH_CURL = YesPlease HAVE_LIBCHARSET_H = YesPlease +HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] Generalize the inclusion of strings.h
The header strings.h was formerly only included for a particular platform to define strcasecmp, but another platform requiring this inclusion has been found. The build system will now include the file based on its presence determined by configure. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 6 ++ configure.ac | 6 ++ git-compat-util.h | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 735a834..fb78f7f 100644 --- a/Makefile +++ b/Makefile @@ -74,6 +74,8 @@ all:: # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks # d_type in struct dirent (Cygwin 1.5, fixed in Cygwin 1.7). # +# Define HAVE_STRINGS_H if you have strings.h and need it for strcasecmp. +# # Define NO_STRCASESTR if you don't have strcasestr. # # Define NO_MEMMEM if you don't have memmem. @@ -1892,6 +1894,10 @@ ifdef HAVE_LIBCHARSET_H EXTLIBS += $(CHARSET_LIB) endif +ifdef HAVE_STRINGS_H +BASIC_CFLAGS += -DHAVE_STRINGS_H +endif + ifdef HAVE_DEV_TTY BASIC_CFLAGS += -DHAVE_DEV_TTY endif diff --git a/configure.ac b/configure.ac index 3347c17..4176db8 100644 --- a/configure.ac +++ b/configure.ac @@ -886,6 +886,12 @@ AC_CHECK_HEADER([libcharset.h], [HAVE_LIBCHARSET_H=YesPlease], [HAVE_LIBCHARSET_H=]) GIT_CONF_SUBST([HAVE_LIBCHARSET_H]) +# +# Define HAVE_STRINGS_H if you have strings.h +AC_CHECK_HEADER([strings.h], +[HAVE_STRINGS_H=YesPlease], +[HAVE_STRINGS_H=]) +GIT_CONF_SUBST([HAVE_STRINGS_H]) # Define CHARSET_LIB if libiconv does not export the locale_charset symbol # and libcharset does CHARSET_LIB= diff --git a/git-compat-util.h b/git-compat-util.h index ace1549..d7359ef 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -99,7 +99,7 @@ #include stdlib.h #include stdarg.h #include string.h -#ifdef __TANDEM /* or HAVE_STRINGS_H or !NO_STRINGS_H? */ +#ifdef HAVE_STRINGS_H #include strings.h /* for strcasecmp() */ #endif #include errno.h -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Declare that HP NonStop systems require strings.h
Am 14.12.2012 20:57, schrieb David Michael: This platform previously included strings.h automatically. However, the build system now requires an explicit option to do so. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fb78f7f..e84b0cb 100644 --- a/Makefile +++ b/Makefile @@ -1357,6 +1357,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # Added manually, see above. NEEDS_SSL_WITH_CURL = YesPlease HAVE_LIBCHARSET_H = YesPlease +HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes If NONSTOP_KERNEL is the platform that defines __TANDEM, then this should be squashed into the previous patch, shouldn't it? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/3] compiling git with gcc -O3 -Wuninitialized
I always compile git with gcc -Wall -Werror, because it catches a lot
of dubious constructs, and we usually keep the code warning-free.
However, I also typically compile with -O0 because I end up debugging
a fair bit.
Sometimes, though, I compile with -O3, which yields a bunch of new
variable might be used uninitialized warnings. What's happening is
that as functions get inlined, the compiler can do more static analysis
of the variables. So given two functions like:
int get_foo(int *foo)
{
if (something_that_might_fail() 0)
return error(unable to get foo);
*foo = 0;
return 0;
}
void some_fun(void)
{
int foo;
if (get_foo(foo) 0)
return -1;
printf(foo is %d\n, foo);
}
If get_foo() is not inlined, then when compiling some_fun, gcc sees only
that a pointer to the local variable is passed, and must assume that it
is an out parameter that is initialized after get_foo returns.
However, when get_foo() is inlined, the compiler may look at all of the
code together and see that some code paths in get_foo() do not
initialize the variable. And we get the extra warnings.
In some cases, this can actually reveal real bugs. The first patch fixes
such a bug:
[1/3]: remote-testsvn: fix unitialized variable
In most cases, though (including the example above), it's a false
positive. We know something the compiler does not: that error() always
returns -1, and therefore we will either exit early from some_fun, or
foo will be properly initialized.
The second two patches:
[2/3]: inline error functions with constant returns
[3/3]: silence some -Wuninitialized warnings around errors
try to expose that return value more clearly to the calling code. After
applying both, git compiles cleanly with -Wall -O3. But the patches
themselves are kind of ugly.
Patch 2/3 tries inlining error() and a few other functions, which lets
the caller see the return value. Unfortunately, this doesn't actually
work in all cases. I think what is happening is that because error() is
a variadic function, gcc refuses to inline it (and if you give it the
always_inline attribute, it complains loudly). So it works for some
functions, but not for error(), which is the most common one.
Patch 3/3 takes a more heavy-handed approach, and replaces some
instances of return error(...) with error(...); return -1. This
works, but it's kind of ugly. The whole point of error()'s return code
is to allow the return error(...) shorthand, and it basically means we
cannot use it in some instances.
I really like keeping us -Wall clean (because it means when warnings do
come up, it's easy to pay attention to them). But I feel like patch 3 is
making the code less readable just to silence the false positives. We
can always use the int foo = foo trick, but I'd like to avoid that
where we can. Not only is it ugly in itself, but it means that we've
shut off the warnings if a problem is ever introduced to that spot.
Can anybody think of a clever way to expose the constant return value of
error() to the compiler? We could do it with a macro, but that is also
out for error(), as we do not assume the compiler has variadic macros. I
guess we could hide it behind #ifdef __GNUC__, since it is after all
only there to give gcc's analyzer more information. But I'm not sure
there is a way to make a macro that is syntactically identical. I.e.,
you cannot just replace error(...) in return error(...); with a
function call plus a value for the return statement. You'd need
something more like:
#define RETURN_ERROR(fmt, ...) \
do { \
error(fmt, __VA_ARGS__); \
return -1; \
} while(0) \
which is awfully ugly.
Thoughts?
-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
[PATCH 1/3] remote-testsvn: fix unitialized variable
In remote-test-svn, there is a parse_rev_note function to parse lines of the form Revision-number from notes. If it finds such a line and parses it, it returns 0, copying the value into a struct rev_note. If it finds an entry that is garbled or out of range, it returns -1 to signal an error. However, if it does not find any Revision-number line at all, it returns success but does not put anything into the rev_note. So upon a successful return, the rev_note may or may not be initialized, and the caller has no way of knowing. gcc does not usually catch the use of the unitialized variable because the conditional assignment happens in a separate function from the point of use. However, when compiling with -O3, gcc will inline parse_rev_note and notice the problem. We can fix it by returning -1 when no note is found (so on a zero return, we always found a valid value). Signed-off-by: Jeff King p...@peff.net --- I think this is the right fix, but I am not too familiar with this code, so I might be missing a case where a missing Revision-number should provide some sentinel value (like 0) instead of returning an error. In fact, of the two callsites, one already does such a zero-initialization. remote-testsvn.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/remote-testsvn.c b/remote-testsvn.c index 51fba05..5ddf11c 100644 --- a/remote-testsvn.c +++ b/remote-testsvn.c @@ -90,10 +90,12 @@ static int parse_rev_note(const char *msg, struct rev_note *res) if (end == value || i 0 || i UINT32_MAX) return -1; res-rev_nr = i; + return 0; } msg += len + 1; } - return 0; + /* didn't find it */ + return -1; } static int note2mark_cb(const unsigned char *object_sha1, -- 1.8.0.2.4.g59402aa -- 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/RFC 2/3] inline error functions with constant returns
The error() function reports an error message to stderr and
returns -1. That makes it handy for returning errors from
functions with a single-line:
return error(something went wrong, ...);
In this case, we know something that the compiler does not,
namely that this is equivalent to:
error(something went wrong, ...);
return -1;
Knowing that the return value is constant can let the
compiler do better control flow analysis, which means it can
give more accurate answers for static warnings, like
-Wuninitialized. But because error() is found in a different
compilation unit, the compiler doesn't get to see the code
when making decisions about the caller.
This patch makes error(), along with a handful of functions
which wrap it, an inline function, giving the compiler the
extra information. This prevents some false positives when
-Wunitialized is used with -O3.
Signed-off-by: Jeff King p...@peff.net
---
Not really meant for inclusion. The opterror() bit does silence one
warning, but I think the error() inlining is doing absolutely nothing.
cache.h | 10 +-
config.c | 9 -
git-compat-util.h | 13 -
parse-options.c | 11 ++-
parse-options.h | 8 +++-
usage.c | 12 +---
6 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/cache.h b/cache.h
index 18fdd18..fb7c5e2 100644
--- a/cache.h
+++ b/cache.h
@@ -1135,10 +1135,18 @@ extern const char *get_commit_output_encoding(void);
extern int check_repository_format_version(const char *var, const char *value,
void *cb);
extern int git_env_bool(const char *, int);
extern int git_config_system(void);
-extern int config_error_nonbool(const char *);
extern const char *get_log_output_encoding(void);
extern const char *get_commit_output_encoding(void);
+/*
+ * Call this to report error for your variable that should not
+ * get a boolean value (i.e. [my] var means true).
+ */
+static inline int config_error_nonbool(const char *var)
+{
+ return error(Missing value for '%s', var);
+}
+
extern int git_config_parse_parameter(const char *, config_fn_t fn, void
*data);
struct config_include_data {
diff --git a/config.c b/config.c
index fb3f868..ea4a98f 100644
--- a/config.c
+++ b/config.c
@@ -1655,12 +1655,3 @@ int git_config_rename_section(const char *old_name,
const char *new_name)
{
return git_config_rename_section_in_file(NULL, old_name, new_name);
}
-
-/*
- * Call this to report error for your variable that should not
- * get a boolean value (i.e. [my] var means true).
- */
-int config_error_nonbool(const char *var)
-{
- return error(Missing value for '%s', var);
-}
diff --git a/git-compat-util.h b/git-compat-util.h
index 2e79b8a..c38de42 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -285,9 +285,20 @@ extern void warning(const char *err, ...)
__attribute__((format (printf, 1, 2)))
extern NORETURN void usagef(const char *err, ...) __attribute__((format
(printf, 1, 2)));
extern NORETURN void die(const char *err, ...) __attribute__((format (printf,
1, 2)));
extern NORETURN void die_errno(const char *err, ...) __attribute__((format
(printf, 1, 2)));
-extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning(const char *err, ...) __attribute__((format (printf, 1,
2)));
+extern void (*error_routine)(const char *err, va_list params);
+
+__attribute__((format (printf, 1, 2)))
+static inline int error(const char *err, ...)
+{
+ va_list params;
+ va_start(params, err);
+ error_routine(err, params);
+ va_end(params);
+ return -1;
+}
+
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err,
va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
diff --git a/parse-options.c b/parse-options.c
index c1c66bd..5268d4e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -18,13 +18,14 @@ int opterror(const struct option *opt, const char *reason,
int flags)
return error(BUG: switch '%c' %s, opt-short_name, reason);
}
-int opterror(const struct option *opt, const char *reason, int flags)
+void opterror_report(const struct option *opt, const char *reason, int flags)
{
if (flags OPT_SHORT)
- return error(switch `%c' %s, opt-short_name, reason);
- if (flags OPT_UNSET)
- return error(option `no-%s' %s, opt-long_name, reason);
- return error(option `%s' %s, opt-long_name, reason);
+ error(switch `%c' %s, opt-short_name, reason);
+ else if (flags OPT_UNSET)
+ error(option `no-%s' %s, opt-long_name, reason);
+ else
+ error(option `%s' %s, opt-long_name, reason);
}
static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
diff --git a/parse-options.h b/parse-options.h
index 71a39c6..23673c7 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -176,7 +176,13 @@ extern int
[PATCH/RFC 3/3] silence some -Wuninitialized warnings around errors
When git is compiled with gcc -Wuninitialized -O3, some
inlined calls provide an additional opportunity for the
compiler to do static analysis on variable initialization.
For example, with two functions like this:
int get_foo(int *foo)
{
if (something_that_might_fail() 0)
return error(unable to get foo);
*foo = 0;
return 0;
}
void some_fun(void)
{
int foo;
if (get_foo(foo) 0)
return -1;
printf(foo is %d\n, foo);
}
If get_foo() is not inlined, then when compiling some_fun,
gcc sees only that a pointer to the local variable is
passed, and must assume that it is an out parameter that
is initialized after get_foo returns.
However, when get_foo() is inlined, the compiler may look at
all of the code together and see that some code paths in
get_foo() do not initialize the variable. As a result, it
prints a warning. But what the compiler can't see is that
error() always returns -1, and therefore we know that either
we return early from some_fun, or foo ends up initialized,
and the code is safe. The warning is a false positive.
By converting the error() call to:
error(unable to get foo);
return -1;
we explicitly make the compiler aware of the constant return
value, and it silences the warning.
Signed-off-by: Jeff King p...@peff.net
---
Not sure if we should do this or not. This does silence the warnings,
but it's kind of ugly.
builtin/reflog.c | 6 --
vcs-svn/svndiff.c | 6 --
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..c2ea9d3 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -494,8 +494,10 @@ static int parse_expire_cfg_value(const char *var, const
char *value, unsigned l
static int parse_expire_cfg_value(const char *var, const char *value, unsigned
long *expire)
{
- if (!value)
- return config_error_nonbool(var);
+ if (!value) {
+ config_error_nonbool(var);
+ return -1;
+ }
if (!strcmp(value, never) || !strcmp(value, false)) {
*expire = 0;
return 0;
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index 74c97c4..46e96f6 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -121,7 +121,8 @@ static int read_int(struct line_buffer *in, uintmax_t
*result, off_t *len)
*len = sz - 1;
return 0;
}
- return error_short_read(in);
+ error_short_read(in);
+ return -1;
}
static int parse_int(const char **buf, size_t *result, const char *end)
@@ -140,7 +141,8 @@ static int parse_int(const char **buf, size_t *result,
const char *end)
*buf = pos + 1;
return 0;
}
- return error(invalid delta: unexpected end of instructions section);
+ error(invalid delta: unexpected end of instructions section);
+ return -1;
}
static int read_offset(struct line_buffer *in, off_t *result, off_t *len)
--
1.8.0.2.4.g59402aa
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Declare that HP NonStop systems require strings.h
Johannes Sixt wrote: Am 14.12.2012 20:57, schrieb David Michael: This platform previously included strings.h automatically. However, the build system now requires an explicit option to do so. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fb78f7f..e84b0cb 100644 --- a/Makefile +++ b/Makefile @@ -1357,6 +1357,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # Added manually, see above. NEEDS_SSL_WITH_CURL = YesPlease HAVE_LIBCHARSET_H = YesPlease +HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes If NONSTOP_KERNEL is the platform that defines __TANDEM, then this should be squashed into the previous patch, shouldn't it? Patch 4/4 does not work without 3/4, Not for HP-NonStop. Bye, Jojo -- 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: possible Improving diff algoritm
* Javier Domingo javier...@gmail.com [121214 13:20]:
I think the idea of being preferable to have a blank line at the end
of the added/deleted block is key in this case.
For symmetry I'd suggest to make it preferable to have blank lines
at the end or the beginning.
{
old
+ }
+
+ {
+ new
}
vs
{
old
}
+
+ {
+ new
+ }
is just the same case in blue.
(Although empty lines alone feels not quite optimal, it is at least a
good start).
Bernhard R. Link
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Declare that HP NonStop systems require strings.h
Am 14.12.2012 23:46, schrieb Joachim Schmitz: Johannes Sixt wrote: Am 14.12.2012 20:57, schrieb David Michael: This platform previously included strings.h automatically. However, the build system now requires an explicit option to do so. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fb78f7f..e84b0cb 100644 --- a/Makefile +++ b/Makefile @@ -1357,6 +1357,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # Added manually, see above. NEEDS_SSL_WITH_CURL = YesPlease HAVE_LIBCHARSET_H = YesPlease +HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes If NONSTOP_KERNEL is the platform that defines __TANDEM, then this should be squashed into the previous patch, shouldn't it? Patch 4/4 does not work without 3/4, Not for HP-NonStop. That is clear from the order of the patches in the series. To put it in a different way: Do all supported platforms still work after 3/4, but without 4/4? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Support builds when sys/param.h is missing
David Michael fedora@gmail.com writes: An option is added to the Makefile to skip the inclusion of sys/param.h. The only known platform with this condition thus far is the z/OS UNIX System Services environment. Signed-off-by: David Michael fedora@gmail.com --- Hmm, makes me wonder if we can remove that inclusion everywhere instead. What definitions are we getting from it? 8695c8b (Add show-files command to show the list of managed (or non-managed) files., 2005-04-11) added it to show-files, and without the include the compilation failed due to lack of MAXPATHLEN, but you shouldn't be using that in the first place (we tend to either use PATH_MAX or lift the upper limit using strbuf these days.). 65bb491 (Add the ability to prefix something to the pathname to checkout-cache.c, 2005-04-21) does the same to checkout-cache.c to grab MAXPATHLEN. bb233d6 (Add support for a GIT_INDEX_FILE environment variable., 2005-04-21) moves these two inclusions to cache.h; removing it bteaks compilation due to lack of MAXPATHLEN, ULONG_MAX, etc., I didn't check what else sys/parms.h was accicdentally slurping into the compilation, but ULONG_MAX is what we explicitly ask for by including either inttypes or stdint these days. Later 4050c0d (Clean up compatibility definitions., 2005-12-05) further consolidated the headers by moving inclusion around. This commit did not audit unused headers. I have this suspicion that nobody would notice if we simply stopped including the header. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Support builds when sys/param.h is missing
Hi, On Fri, Dec 14, 2012 at 6:41 PM, Junio C Hamano gits...@pobox.com wrote: I have this suspicion that nobody would notice if we simply stopped including the header. While I'm not aware of any subtleties it could be causing on other platforms, it does seem fine to drop sys/param.h on my test GNU/Linux systems. I can resend the series and just remove the include, if preferred. Thanks. David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] cache-tree: invalidate i-t-a paths after generating trees
On Thu, Dec 13, 2012 at 9:04 AM, Junio C Hamano gits...@pobox.com wrote:
@@ -324,7 +325,14 @@ static int update_one(struct cache_tree *it,
if (!sub)
die(cache-tree.c: '%.*s' in '%s' not found,
entlen, path + baselen, path);
- i += sub-cache_tree-entry_count - 1;
+ i--; /* this entry is already counted in sub */
Huh?
The -1 in the original is the bog-standard compensation for the
for(;;i++) loop.
Exactly. It took me a while to figure out what - 1 was for and I
wanted to avoid that for other developers. Only I worded it badly.
I'll replace the for loop with a while loop to make it clearer...
+ if (sub-cache_tree-entry_count 0) {
+ i -= sub-cache_tree-entry_count;
+ sub-cache_tree-entry_count = -1;
+ to_invalidate = 1;
+ }
+ else
+ i += sub-cache_tree-entry_count;
While the rewritten version is not *wrong* per-se, I have a feeling
that it may be much easier to read if written like this:
if (sub-cache_tree_entry_count 0) {
to_invalidate = 1;
to_skip = 0 - sub-cache_tree_entry_count;
sub-cache_tree_entry_count = -1;
} else {
to_skip = sub-cache_tree_entry_count;
}
i += to_skip - 1;
..or this would be fine too. Which way to go?
A while we're still at the cache tree
- if (ce-ce_flags (CE_REMOVE | CE_INTENT_TO_ADD))
- continue; /* entry being removed or placeholder */
+ /*
+* CE_REMOVE entries are removed before the index is
+* written to disk. Skip them to remain consistent
+* with the future on-disk index.
+*/
+ if (ce-ce_flags CE_REMOVE)
+ continue;
+
A CE_REMOVE entry is removed later from the index, however it is still
counted in entry_count. entry_count serves two purposes: to skip the
number of processed entries in the in-memory index, and to record the
number of entries in the on-disk index. These numbers do not match
when CE_REMOVE is present. We have correct in-memory entry_count,
which means incorrect on-disk entry_count in this case.
I tested with an index that has a/b and a/c. The latter has CE_REMOVE.
After writing cache tree I get:
$ git ls-files --stage
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 a/b
$ ../test-dump-cache-tree
878e27c626266ac04087a203e4bdd396dcf74763 (2 entries, 1 subtrees)
878e27c626266ac04087a203e4bdd396dcf74763 #(ref) (1 entries, 1 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a a/ (2 entries, 0 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a #(ref) a/ (1 entries, 0 subtrees)
If I throw out that index, create a new one with a/b alone and write-tree, I get
$ ../test-dump-cache-tree
878e27c626266ac04087a203e4bdd396dcf74763 (1 entries, 1 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a a/ (1 entries, 0 subtrees)
Shall we fix this too? I'm thinking of adding skip argument to update_one and
i += sub-cache_tree-entry_count - 1;
would become
i += sub-cache_tree-entry_count + skip - 1;
and entry_count would always reflect on-disk value. This skip can be
reused for this i-t-a patch as well.
--
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/RFC 0/3] compiling git with gcc -O3 -Wuninitialized
On Sat, Dec 15, 2012 at 5:09 AM, Jeff King p...@peff.net wrote:
I always compile git with gcc -Wall -Werror, because it catches a lot
of dubious constructs, and we usually keep the code warning-free.
However, I also typically compile with -O0 because I end up debugging
a fair bit.
Sometimes, though, I compile with -O3, which yields a bunch of new
variable might be used uninitialized warnings. What's happening is
that as functions get inlined, the compiler can do more static analysis
of the variables. So given two functions like:
int get_foo(int *foo)
{
if (something_that_might_fail() 0)
return error(unable to get foo);
*foo = 0;
return 0;
}
void some_fun(void)
{
int foo;
if (get_foo(foo) 0)
return -1;
printf(foo is %d\n, foo);
}
If get_foo() is not inlined, then when compiling some_fun, gcc sees only
that a pointer to the local variable is passed, and must assume that it
is an out parameter that is initialized after get_foo returns.
However, when get_foo() is inlined, the compiler may look at all of the
code together and see that some code paths in get_foo() do not
initialize the variable. And we get the extra warnings.
Other options:
- Any __attribute__ or #pragma to aid flow analysis (or would gcc dev be
willing to add one)?
- Maintain a list of false positives and filter them out from gcc output?
And if we do this, should we support other compilers as well? I tried
clang once a long while ago and got a bunch of warnings iirc.
--
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: What's cooking in git.git (Dec 2012, #03; Wed, 12)
On Fri, Dec 14, 2012 at 7:11 AM, Max Horn post...@quendi.de wrote: please stop referring to facts and obvious. You pretend to be a being of pure reason and that everything you say is logical, drawn from facts. But you forget or perhaps do not know that logic by itself proofs nothing, it all depends on the axioms you impose. And yours are quite different from what others consider as such, and apparently also inconsistent. When I say something is a fact, I do mean it. It's not my opinion, it's not what I think, it's a fact. The Earth revolves around the Sun, Junio is the maintainer, Junio can merge whatever he wants, those are facts. If you think the Earth revolving around the Sun is not a fact, that's your choice, but if you want to convince others that this is not a fact, you need to show *why*. So, instead of trying to twist things around so that broken things in your code are not broken after all, why not simply re-roll your patch with the obvious fixes applies? First of all, it depends on your definition of broken. To me something broken is something that does not *work*. The code works, the tests have some cruft in it, but it *works*, it's not broken. And I said multiple times now that I WILL FIX IT. I'm not going to repeat it again, and quite frankly, I don't think I'm going to reply to this thread any more. What makes you think that you owe my free time? I do with my free time whatever I want. I will fix it, when I feel like fixing it. This code will not go into 1.8.1, so there's no point hurrying the fix, I have other things to do. As you write yourself, time is not pressing at all -- so I don't see why your patch should be merged now, and fixed later, contrary to how other people's patches are treated? Why not fix them first, and then apply? We do have time, after all! And nobody is expecting you to do that while you are on vacation, either. Nor that you do it instantly. Time is not pressing because Junio decided not to merge, and he decided not to merge, because of this issue. This hurts users, and benefits no one. Just say: OK, I see there is a problem with the patches; even though I consider it unimportant, I will play by the rules everybody here has to follow, and re-roll the patch series. But this is of low priority for me, so I cannot say right now when it will happen. That is what I said. What part of I didn't say it was irrelevant, it should be fixed didn't you understand? And no, this is not a rule, you assume it *must* be re-rolled, it doesn't. Everybody would be happy then. Except perhaps the hypothetical users, who would have to wait a bit longer -- but oh, not really, because they can just use remote-bzr from your repo, yay :-). I really like that about it, it lives in contrib, so one can use it w/o it being merged into git.git. It's not because of me that users would have to wait, it was Junio's decision, there was literally nothing I could do, before, or now, for this code to be merged for 1.8.1. And yes, people can use my repo *if* they know about it, most people just follow the mainline, and many don't even read the news. Instead, you make claims that make you look like a foolish and arrogant ass, all the while insulting Junio and me implicitly. Why do you do that??? It delays acceptance of your nice work. As you write, this hurts the users. So why do it? Show me *exactly* where I insulted anybody, and show me where I made a false claim. And no, this discussion doesn't delay the acceptance, the acceptance was already delayed before this discussion, there was nothing I could do, neither in the past or present. Since you keep complaining that nobody ever really can point to anything wrong your said, I'll do you the favor by deconstructing one of the claims you made: Please. On 13.12.2012, at 20:06, Felipe Contreras wrote: On Thu, Dec 13, 2012 at 6:04 AM, Max Horn post...@quendi.de wrote: On 13.12.2012, at 11:08, Felipe Contreras wrote: On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: New remote helper for bzr (v3). With minor fixes, this may be ready for 'next'. What minor fixes? Lookng at the above (fixup), $gmane/210744 comes to mind That doesn't matter. The code and the tests would work just fine. It doesn't matter? I find that statement hard to align with what the maintainer of git, and thus the person who decides whether your patch series gets merged or not, wrote just above? In fact, it seems to me that what Junio said matters a great deal... So you think Junio knows more about remote-bzr than I do? This is a classical straw man argument. It was not a straw man argument, in fact, it's not an argument at all, it's a *question*. No, I do not think that. But I do think that Junio knows enough to review your code, and I do think that the point he raised is valid. You disagree with the importance of his
Re: FW: Git log --graph doesn't output color when redirected
On Thu, Dec 13, 2012 at 8:13 PM, Jeff King p...@peff.net wrote: If you are using --format=%C(red) or similar placeholders, they are the odd duck by not respecting the auto-color mode. But they should, shouldn't they? Just asking. I may do it to when I revive nd/pretty-placeholder-with-color-option. -- 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
Unexpected behaviour when trying to merge two new repositories.
Hi, I have a directory of files that have been copied between different machines, and then drifted apart. I was trying to merge the different versions into a single unified repository. I succeeded, but I had to navigate though, and recover from, what seems to me to be counter-intuitive behaviour. Let me walk through a mock up of what I did. Scenario 1. Merging two repositories. Mock up first directory, and commit it. mkdir a git init a Initialized empty Git repository in /tmp/a/.git/ cd a echo 1 1 echo 2 2 git add . git commit -m add 1 and 2 [master (root-commit) 9a649ab] add 1 and 2 2 files changed, 2 insertions(+), 0 deletions(-) create mode 100644 1 create mode 100644 2 Mock up second directory. Add files, but don't commit. mkdir b cd ../b git init Initialized empty Git repository in /tmp/b/.git/ echo 1 1 echo 3 3 git pull ../a remote: Counting objects: 4, done. remote: Compressing objects: 100% (2/2), done. remote: Total 4 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (4/4), done. From ../a * branchHEAD - FETCH_HEAD error: Untracked working tree file '1' would be overwritten by merge. git add 1 3 Now merge in the first directory. git pull ../a master From ../a * branchmaster - FETCH_HEAD Already up-to-date. git status # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # #deleted:2 #new file: 3 # This surprised me. I wasn't expecting 2 to be deleted. And I wasn't expecting to be told that it was Already up to date. Scenario 2. Merging two repositories with everything committed. Mock up a third directory. Add files and this time, commit. Then merge from a. mkdir ../c cd ../c git init Initialized empty Git repository in /tmp/c/.git/ echo 1 1 echo 4 4 git add 1 4 git commit -m add 1 and 4 [master (root-commit) b6081d9] add 1 and 4 2 files changed, 2 insertions(+), 0 deletions(-) create mode 100644 1 create mode 100644 4 git pull ../a master warning: no common commits remote: Counting objects: 4, done. remote: Compressing objects: 100% (2/2), done. remote: Total 4 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (4/4), done. From ../a * branchmaster - FETCH_HEAD Merge made by recursive. 2 |1 + 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 2 ll * -rw-rw-r-- 1 ben ben 2 2012-12-15 15:02 1 -rw-rw-r-- 1 ben ben 2 2012-12-15 15:10 2 -rw-rw-r-- 1 ben ben 2 2012-12-15 15:03 4 This is what I was expecting the first time. It seems that if there are no commits in the new repository, git doesn't detect that there are no common commits. Scenario 3. Merge with uncommitted changes. Create another mock directory. Create some of the same files, but with different contents. Again, add them but don't commit. mkdir ../d cd ../d git init Initialized empty Git repository in /tmp/d/.git/ echo 1a 1 echo 5 5 git add 1 5 git commit -m add 1 and 5 [master (root-commit) b6081d9] add 1 and 5 2 files changed, 2 insertions(+), 0 deletions(-) create mode 100644 1 create mode 100644 5 git pull ../a master remote: Counting objects: 4, done. remote: Compressing objects: 100% (2/2), done. remote: Total 4 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (4/4), done. From ../a * branchmaster - FETCH_HEAD git status # On branch master nothing to commit (working directory clean) ls -l total 8 -rw-rw-r-- 1 ben ben 2 2012-12-15 15:31 1 -rw-rw-r-- 1 ben ben 2 2012-12-15 15:31 2 This too, surprised me. This time, the incoming file was added where before it was deleted. And file 5 has vanished. git fsck dangling blob a8994dc188ebbbc9e8a885470651a7fbb9127528 dangling blob 7ed6ff82de6bcc2a78243fc9c54d3ef5ac14da69 cat 1 1 git cat-file -p a8994dc188ebbbc9e8a885470651a7fbb9127528 1a git cat-file -p 7ed6ff82de6bcc2a78243fc9c54d3ef5ac14da69 5 In fact, both file 1 and file 5 have been 'lost'. It's not obvious to me why this happens, except that it seems to be a feature of the need to merge. This seems inconsistent. In one case, we have merged all 3 files. In one case, we have deleted incoming files. And in one case, we have 'lost' files that were already in the repository. In one way, this is a slightly obscure edge case - merging into a new repository and then adding but not committing the files. But the people most likely to do this are new git users - the ones who have to work hardest to recover the deleted/lost files. Regards, Ben -- 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 2012, #03; Wed, 12)
On Sat, Dec 15, 2012 at 1:09 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 12/15/2012 04:14 AM, Felipe Contreras wrote: I'm going to say it one last time; merging this patch series either creates issues for the users, or not. There is a reality out there, independent of what you, Junio, or me think or say. And the fact is, that if this patch series is going to create issues for the users, *nobody* has pointed out why, so, since there's no evidence for it, the only rational thing to do is believe that there will be no issues for the users. There is no known issue with the code, that is a fact. This code could be easily merged today, and in fact, it was merged by Junio already (but then reverted). There are no positive outcomes from the delay, only negative ones. I will address the minute issue about the extra cruft, eventually. Couple of facts first: a) This code was already merged b) This code is for a test c) I'm the only developer so far Cruft in the codebase is a problem for git *developers* because it makes the code harder to maintain and extend. A problem big enough to warrant the rejection of the patch series? No. And therefore cruft is a problem for git *users* because it slows down future development (in whatever small amount). Don't confuse potential issues with real ones. It *might* slow down future development, but will it do it with absolute certainty and beyond any reasonable doubt? No, it might not slow anything at all. And even if it does, by how much? 50%? 10%? 1%? Chances are it would be barely noticeable to the users. And even if it was substantial, this is on *test* code. Most users survive just fine with most of the contrib code not having tests at all, they can probably survive with the development of the test code for remote-bzr being a tad slower. But who are these developers that would be slowed down? So far I'm the only contributor, and I'm not going to be slowed. If and when somebody else contributes, and find his or her development is slowed down by this, he or her would probably start by removing that code his or herself, and submit the appropriate patch. Moreover, it is dangerous for a project to accept crufty code based on a contributor's promise to clean up the code later: But it was already accepted: http://git.kernel.org/?p=git/git.git;a=commitdiff;h=ad38af72b334150e6cf1978721c37077ae3c6d7f The world didn't end the first time, presumably, if this code is accepted again, the world will not end either. * The developer might not get around to it, or might take longer than expected. Somebody else could do it. This is collaborative development after all, is it not? I don't see people halting because something is somebody else's code. * Until it is cleaned up, the cruft hinders other potential developers to that code. How many *potential* developers are we talking about? By how much? * The presence of cruft lowers the expectation of quality for the whole project; cruft breeds more cruft. Please. This is in test code for the contrib area, most code in that area doesn't even have tests. It is simpler and fairer to have a policy no crufty code than to try to evaluate each instance on a case-by-case basis. Even then, the problem can be easily solved by simply removing the whole file (contrib/remote-helpers/test-bzr.sh), I say that has more potential to hurt users and developers, but hey, no crufty code. Since most code in the contrib area doesn't have tests, we would still be following the policy. None of this benefits the *real* users one iota. Anyway, these theoretical minute problems aren't worth worrying about, nor discussing. If you want to damage real users, go ahead. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
