* [PATCH 1/4] Support builds when sys/param.h is missing
From: David Michael @ 2012-12-14 19:56 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: gitster
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.dm0@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
^ permalink raw reply related
* [PATCH 2/4] Detect when the passwd struct is missing pw_gecos
From: David Michael @ 2012-12-14 19:56 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: gitster
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.dm0@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
^ permalink raw reply related
* Re: Build fixes for another obscure Unix
From: David Michael @ 2012-12-14 19:48 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <kaem06$3go$1@ger.gmane.org>
Hi,
On Fri, Dec 14, 2012 at 2:54 AM, Joachim Schmitz
<jojo@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 <jojo@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
^ permalink raw reply
* [PATCH 2/2] Port to QNX
From: Matt Kraai @ 2012-12-14 18:38 UTC (permalink / raw)
To: git; +Cc: Matt Kraai
In-Reply-To: <1355510300-31541-1-git-send-email-kraai@ftbfs.org>
From: Matt Kraai <matt.kraai@amo.abbott.com>
Signed-off-by: Matt Kraai <matt.kraai@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
^ permalink raw reply related
* [PATCH 0/2] Port to QNX
From: Matt Kraai @ 2012-12-14 18:38 UTC (permalink / raw)
To: git
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
^ permalink raw reply
* [PATCH 1/2] Make lock local to fetch_pack
From: Matt Kraai @ 2012-12-14 18:38 UTC (permalink / raw)
To: git; +Cc: Matt Kraai
In-Reply-To: <1355510300-31541-1-git-send-email-kraai@ftbfs.org>
From: Matt Kraai <matt.kraai@amo.abbott.com>
lock is only used by fetch_pack, so move it into that function.
Signed-off-by: Matt Kraai <matt.kraai@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
^ permalink raw reply related
* Re: [PATCH] Documentation/git: add missing info about --git-dir command-line option
From: Manlio Perillo @ 2012-12-14 17:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwxj3vxx.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 12/12/2012 20:35, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@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-----
^ permalink raw reply
* [PATCH] README: Git is released under the GPLv2, not just "the GPL"
From: Stefano Lattarini @ 2012-12-14 15:37 UTC (permalink / raw)
To: git; +Cc: gitster
And this is clearly stressed by Linus in the COPYING file. So make it
clear in the README as well, to avoid possible misunderstandings.
Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
README | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/README b/README
index d2690ec..c50e6f4 100644
--- a/README
+++ b/README
@@ -19,9 +19,10 @@ Git is a fast, scalable, distributed revision control system with an
unusually rich command set that provides both high-level operations
and full access to internals.
-Git is an Open Source project covered by the GNU General Public License.
-It was originally written by Linus Torvalds with help of a group of
-hackers around the net. It is currently maintained by Junio C Hamano.
+Git is an Open Source project covered by the GNU General Public License
+(version 2). It was originally written by Linus Torvalds with help
+of a group of hackers around the net. It is currently maintained by
+Junio C Hamano.
Please read the file INSTALL for installation instructions.
--
1.8.0.1.347.gf94c325
^ permalink raw reply related
* Re: How to avoid the ^M induced by Meld and Git
From: Karl Brand @ 2012-12-14 13:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <50C8AE14.4000704@erasmusmc.nl>
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
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Max Horn @ 2012-12-14 13:11 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <CAMP44s3Es-rLjwe6sgOi9OmwQouM4AbFKAbGB5UgS6sUtYRgKQ@mail.gmail.com>
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 <postbox@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 <gitster@pobox.com> wrote:
>>>> Felipe Contreras <felipe.contreras@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 fail to see a justification in this case... You mentioned "the users" but AFAIK there are no known users yet, and even if, they can simply use remote-bzr from your tree.
This could perhaps be documented explicitly in Documentation/SubmittingPatches. Not sure if I think this would be a good idea or even helpful, just thinking out aloud.
> but Junio said
> "With minor fixes, this may be ready for 'next'." which is no true
> IMO, it's ready *now*, it was ready one month ago. For 'next', this
> problem doesn't matter.
>
> The feedback is appreciated, but delaying the merging of this code for
> no reason makes little sense to me.
And here goes the insult. You say Junio has no reason to delay the merging. When you really mean that you don't agree with his reasons. So you attack his professionalism and integrity by alluding that he has some ulterior motives to delay the patch. E.g. that he is hates you, is just mean, does it out of stubbornness, etc.
> Junio, of course, can do whatever he wants. The removal of this no-op code can wait, or it can be done
> on top of v3, there's no need for re-roll, and Junio already
> complained about the v3 re-roll.
>
> And I didn't act because I was on vacations, git development is not my
> only priority.
Of course. Nobody is complaining that you take too long to reply. We are just unhappy in the way you reply when you do reply :-(.
> And even if I had time, I don't see why I should
> prioritize this fix, it's not important, the code is ready.
Another straw man: Nobody asked you to prioritize the fix, take your time. It was you who asked that the series should be applied without any further fixes.
>
>>>> but there may be others. It is the responsibility of a contributor to keep
>>>> track of review comments others give to his or her patches and
>>>> reroll them, so I do not recall every minor details, sorry.
>>>
>>> There is nothing that prevents remote-bzr from being merged.
>>
>> Well, I think that is up to Junio to decide in the end, though :-). He wrote
>
> No. He can decide if the code gets merged, but he is not the voice of
> truth. Nothing prevents him from merging the code, except himself.
> There is no known issue with the code, that is a true fact.
Here are a multitude of fallacies hidden, partially explainable by a differing set of axioms, and/or shear arrogance.
Let us re-reread what was said: Initially, you claimed that "There is nothing that prevents remote-bzr from being merged".
Now, it wasn't said in the above, but let me make it explicit: This statement is "obviously"[1] wrong. There are parts of the patch series Junio thinks are not up to par, and he made it quite clear that he will not merge it until these things are resolved.
Hence, ignoring all else, there obviously *is* something that prevents remote-bzr from being merged. That is a "fact". You even admit so yourself, also contradicting yourself:
"Nothing prevents him from merging the code, except himself."
So how is it possible that you can claim that there is nothing that prevents the merge? Ignoring the self-contradicting aspects of what you wrote, the basis for your differing conclusion seems to be that you change the terms of discussion and are using a different set of axioms. In particular, you apparently redefine
"things that prevent remote-bzr from being merged"
as
"things that in Felipe's view prevent remote-bzr from being merged".
Of course one can arbitrarily bend the rules by this definition. For example, we could redefine "nothing prevents the merge" as
"no technical reasons prevent the merge", and the latter is indeed quite true; your patch series applies perfectly fine, git can do that. Of course the same holds for a patch which removes git.c from the repository, so I don't think this definition is particularly useful...
Back to your self-contradictory statement: It could be parsed as an (not well-formed) attempt to say that Junio has no objective reasons to reject your patch. I.e. you again imply that Junio's decision that the patch is not merge-ready is not based upon "logical conclusions from the given set of facts". Indeed, I would dare say that many people on the list will have interpreted your statements this way... At least I did.
This is something what a lot of people would consider a strong insult towards the professionalism and integrity of Junio. There are more examples of this in previous communications between you and other people in the list.
You finally add "There is no known issue with the code, that is a true fact.". Within your axiom set, this is certainly true. It certainly is not true in mine or Junio's... Yet you very strongly emphasis with your statement that your set of axioms is the correct one to use here, although I would guess that most people would disagree.
This is especially arrogant in view of the another straw man argument you are employing: By writing "[Junio] he is not the voice of truth.", you implicate that I or anybody were of this opinion. But I am not, and what I wrote cannot logically be construed as saying so. At least not within what most people would consider as axiom set; of course if your axiom set includes "Max believes Junio is the voice of truth", your claim because truth, albeit a tautological one. But let me make clear that any such axioms, or set of axioms leading to that implicating, are inconsistent: In my view, of course Junio is fallible and makes mistakes, and can be wrong etc. -- like any human being. Including most definitely me and you.
This is a horrible way of working within a team effort :-(. I find this a great pity, because I believe you are doing some really nice work, I esp. like your remote-hg which works much better for me than the others I tried so far.
Bye,
Max
[1] As a mathematician, I was taught to avoid the word "obvious" in any written form of proof, as it makes you sound arrogant, and it also discourages the reader from thinking critical about a statement, which is considered extremely bad. But since you like it so much, I am using here on purpose.
^ permalink raw reply
* Re: possible Improving diff algoritm
From: Javier Domingo @ 2012-12-14 12:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Geert Bosch, Morten Welinder, Kevin, git
In-Reply-To: <7vzk1izcv6.fsf@alter.siamese.dyndns.org>
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.
Javier Domingo
2012/12/13 Junio C Hamano <gitster@pobox.com>:
> Geert Bosch <bosch@adacore.com> writes:
>
>> It would seem that just looking at the line length (stripped) of
>> the last line, might be sufficient for cost function to minimize.
>> Here the some would be 3 vs 0. In case of ties, use the last
>> possibility with minimum cost.
>
> -- 8< --
> #ifdef A
>
> some stuff
> about A
>
> #endif
> #ifdef Z
>
> some more stuff
> about Z
>
> #endif
> -- >8 --
>
> If you insert a block for M following the existing formatting
> convention in the middle, your heuristics will pick the blank line
> after "about A" as having minimum cost, no?
>
> You inherently have to know the nature of the payload, as your eyes
> that judge the result use that knowledge when doing so, I am afraid.
> I think your "define a function that gives a good score to lines
> that are likely to be good breaking points" idea has merit, but I
> think that should be tied to the content type, most likely via the
> attribute mechanism.
>
> In any case, I consider this as a low-impact (as Michael Haggerty
> noted, it is impossible to introduce a bug that subtly break the
> output; your result is either totally borked or is correct) and
> low-hanging fruit (it can be done as a postprocessing phase after
> the xdiff machinery has done the heavy-lifting of computing LCA), if
> somebody wants to experiment and implement one. As long as the new
> heuristics is hidden behind an explicit command line option to avoid
> other "consequences", I wouldn't discourage interested parties from
> working on it. It is not just my itch, though.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] git.c: add --index-file command-line option.
From: Manlio Perillo @ 2012-12-14 11:23 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
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.perillo@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 --index-file=$index_file ls-tree HEAD)"
+
+ ) &&
+ cmp .git/index saved-index >/dev/null
+'
+
cat > expect << EOF
zort
--
1.8.1.rc1.17.g75ed918.dirty
^ permalink raw reply related
* Re: Build fixes for another obscure Unix
From: Joachim Schmitz @ 2012-12-14 7:54 UTC (permalink / raw)
To: git
In-Reply-To: <CAEvUa7nNNYREAsxc==tfg+e1XNZFbDVOpGXE6z-7+SfbqNrp6Q@mail.gmail.com>
David Michael wrote:
> Hi,
>
> On Thu, Dec 13, 2012 at 12:18 PM, Pyeron, Jason J CTR (US)
> <jason.j.pyeron.ctr@mail.mil> wrote:
>>> Would there be any interest in applying such individual
>>> compatibility fixes for this system, even if a full port doesn't
>>> reach completion?
>>
>> What are the down sides? Can your changes be shown to not impact
>> builds on other systems?
>
> I've pushed a handful of small compatibility patches to GitHub[1]
> which are enough to successfully compile the project. The default
> values of the new variables should make them unnoticeable to other
> systems.
>
> Are there any concerns with this type of change? If they would be
> acceptable, I can try sending the first four of those patches to the
> list properly. (I expect the last two may be tweaked as I continue
> working with the port.)
>
> I do have a concern with strings.h, though. That file will be
> included for most people who run ./configure, when it wasn't before.
> Do you think it's worth making a more detailed test to see if
> strcasecmp is still undefined after string.h is included, rather than
> just testing for the header's existence?
>
> Thanks.
>
> David
>
> [1] https://github.com/dm0-/git/commits
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 <jojo@schmitz-digital.de>
^ permalink raw reply
* [PATCH] Documentation/git-clean: Document --force --force
From: Soren Brinkmann @ 2012-12-14 1:46 UTC (permalink / raw)
To: git; +Cc: Soren Brinkmann
This patch documents the behavior of 'git clean' when
encountering nested git repositories.
Such repositories are only deleted if '-f' is passed twice
to 'git clean'.
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
Documentation/git-clean.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 9f42c0d..0b31454 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -23,6 +23,9 @@ example, be useful to remove all build products.
If any optional `<path>...` arguments are given, only those paths
are affected.
+Nested git repositories are not removed unless the '-f' option is
+passed to 'git clean' twice.
+
OPTIONS
-------
-d::
@@ -35,6 +38,8 @@ OPTIONS
--force::
If the git configuration variable clean.requireForce is not set
to false, 'git clean' will refuse to run unless given -f or -n.
+ Pass this option twice to 'git clean' in order to also remove
+ nested git repositories.
-n::
--dry-run::
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Felipe Contreras @ 2012-12-14 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vhanpwllh.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 5:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Perhaps. It's not really clear if we should update the working tree at
>>> all. A 'git push' doesn't update the working directory on the remote,
>>> but a 'bzr push' does. I thought it was better to leave this
>>> distinction clear, in case this becomes an issue later on.
>> ...
>> ... I don't mind rephrasing that four lines and amend the
>> log message of what I have in 'pu', if that is what you want.
>
> ... which may look like this.
>
> remote-bzr: update working tree upon pushing
>
> A 'git push' doesn't update the working directory on the remote, but
> a 'bzr push' does. Teach the remote helper for bzr to update the
> working tree on the bzr side upon pushing via the "export" command.
>
> Let me know if I grossly misinterpreted/misrepresented what you
> wanted to do in this patch.
Looks good to me.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-14 0:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsj79wmck.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 5:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> One of the review points were about this piece in the test:
>>>
>>> > +cmd=<<EOF
>>> > +import bzrlib
>>> > +bzrlib.initialize()
>>> > +import bzrlib.plugin
>>> > +bzrlib.plugin.load_plugins()
>>> > +import bzrlib.plugins.fastimport
>>> > +EOF
>>> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>>>
>>> I cannot see how this could have ever worked.
>>>
>>> And I still don't see how your "would work just fine" can be true.
>>
>> As I have explained, all this code is the equivalent of python -c '',
>> or rather, it's a no-op. It works in the sense that it doesn't break
>> anything.
>
> Aren't you ashamed of yourself after having said this?
It is a fact.
>> The purpose of the code is to check for the fastimport plug-in, but
>> that plug-in is not used any more, it's vestigial code, it doesn't
>> matter if the check works or not, as long as it doesn't fail.
>
> If so, the final version that is suitable for merging would have
> that unused code stripped away, no?
To the users there's absolutely no difference.
>>> But it is totally a different matter to merge a crap with known
>>> breakage that is one easy fix away from the get-go. Allowing that
>>> means that all the times we spend on reviewing patches here go
>>> wasted, discouraging reviewers.
>>
>> There is no breakage.
>
> Unused code that burdens others to read through to make sure nothing
> is broken is already broken from maintenance point of view.
Remove the whole test then. I'm already doing way more than most of
the code in contrib by providing tests.
> Why are you wasting my time and everybody's bandwidth on this, when
> you are very well capable of rerolling the series with removal and
> style fixes in far shorter time?
I will do that, when I do that.
We have no time constraints, have we? This code is not getting in
1.8.1 either way.
Anyway, if you merge this code as it is, nothing bad will happen.
Nobody would get hurt, and in fact, very few, if anybody, would
notice.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: unclear documentation of git fetch --tags option and tagopt config
From: Junio C Hamano @ 2012-12-14 0:13 UTC (permalink / raw)
To: Philip Oakley; +Cc: 乙酸鋰, git
In-Reply-To: <084AB408ED4E4CF3B048B8615658F158@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> What would be the best way of updating the documentation to clarify the
> point? Given ch3cooli's previous surprise.
Oh, thanks for bringing it up. I was about to start another message
that begins with "Having said all that..." ;-)
I think the entire paragraph should be rewritten. The first long
sentence explains that you will get tags that point at the branches
and other stuff you follow by default, so there is no need to
explicitly ask for tags most of the time. While that is true, it is
secondary in describing what "--tags" is about. It is to grab all
tags and store them locally, and that needs to come at the very
beginning.
And that auto-following behaviour is already described in the
previous entry for -n/--no-tags. So how about something like this:
This is a short-hand for giving "refs/tags/*:refs/tags/*"
refspec from the command line, to ask all tags to be fetched
and stored locally.
Note that it is deliberate that the above does not mention tagopt
configuration at all. The variable was primarily meant to be used
with --no-tags, so that with this:
[remote "origin"] tagopt = --no-tags
you can "git fetch origin" to keep up with the development on
branches without having to fetch tags from there. Fetching tags and
only tags from a remote is almost always not what you want; in other
words, remote."origin".tagopt set to --tags is a misconfiguration
99% of the time, unless you are only interested in following tagged
release points.
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Junio C Hamano @ 2012-12-13 23:58 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vobhxwm3n.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> Perhaps. It's not really clear if we should update the working tree at
>> all. A 'git push' doesn't update the working directory on the remote,
>> but a 'bzr push' does. I thought it was better to leave this
>> distinction clear, in case this becomes an issue later on.
> ...
> ... I don't mind rephrasing that four lines and amend the
> log message of what I have in 'pu', if that is what you want.
... which may look like this.
remote-bzr: update working tree upon pushing
A 'git push' doesn't update the working directory on the remote, but
a 'bzr push' does. Teach the remote helper for bzr to update the
working tree on the bzr side upon pushing via the "export" command.
Let me know if I grossly misinterpreted/misrepresented what you
wanted to do in this patch.
Thanks.
^ permalink raw reply
* Re: unclear documentation of git fetch --tags option and tagopt config
From: Philip Oakley @ 2012-12-13 23:54 UTC (permalink / raw)
To: Junio C Hamano, 乙酸鋰; +Cc: git
In-Reply-To: <7v7golzta8.fsf@alter.siamese.dyndns.org>
From: "Junio C Hamano" <gitster@pobox.com> Sent: Thursday, December 13,
2012 6:44 PM
> 乙酸鋰 <ch3cooli@gmail.com> writes:
>
>> With git fetch --tags
>> or remote.origin.tagopt = --tags
>> git fetch only fetches tags, but not branches.
>> Current documentation does not mention that no branches are fetched /
>> pulled when --tags option or remote.origin.tagopt = --tags is
>> specified.
>
> In the canonical form you spell out what you want to fetch from
> where, and a lazy "git fetch" or "git fetch origin" that does not
> specify what are to be fetched are the special cases. Because they
> do not say what to fetch, they would become a no-op, which would not
> be very useful, if there is no special casing for them. Instead,
> they use sensible default, taking refspec from the configuration
> variable remote.$name.fetch.
>
> Giving refspecs or the "--tags" option from the command line is a
> way to explicitly override this default, hence:
>
> $ git fetch origin HEAD
>
> only fetches the history leading to the commit at HEAD at the
> remote, ignoring the configured refspecs. As "--tags" is a synonym
> to "refs/tags/*:refs/tags/*", "git fetch --tags origin" tells us to
> ignore refspecs and grab only the tags, i.e.:
>
> $ git fetch origin "refs/tags/*:refs/tags/*"
>
> which does not grab any branches.
>
> You can of course do:
>
> $ git fetch --tags origin
> refs/heads/master:refs/remotes/origin/master
>
> --
What would be the best way of updating the documentation to clarify the
point? Given ch3cooli's previous surprise.
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Junio C Hamano @ 2012-12-13 23:47 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <CAMP44s1ZdMK+0_pP3qkZUepOvkfMaXeY2BV0MFu5YOSV=40Dcw@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>> contrib/remote-helpers/git-remote-bzr | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>>> index 2c05f35..5b89a05 100755
>>> --- a/contrib/remote-helpers/git-remote-bzr
>>> +++ b/contrib/remote-helpers/git-remote-bzr
>>> @@ -571,6 +571,8 @@ def do_export(parser):
>>> repo.generate_revision_history(revid, marks.get_tip('master'))
>>> revno, revid = repo.last_revision_info()
>>> peer.import_last_revision_info_and_tags(repo, revno, revid)
>>> + wt = peer.bzrdir.open_workingtree()
>>> + wt.update()
>>> print "ok %s" % ref
>>> print
>>
>> Shouldn't this be squashed as part of one of the earlier patches?
>> The split between 1/7 (import first) and 2/7 (then support export)
>> makes sense, but this one looks more like "oops, we forgot to touch
>> the working tree while updating the history in the previous steps
>> and here is a fix" to me.
>
> Perhaps. It's not really clear if we should update the working tree at
> all. A 'git push' doesn't update the working directory on the remote,
> but a 'bzr push' does. I thought it was better to leave this
> distinction clear, in case this becomes an issue later on.
If you explained that in the log message, then we would have avoided
this exchange, and more importantly, if you had a in-code comment
there, it may help people who later want to add a knob to leave the
working tree intact. The seed you sow now to help others, who may
be less skillful and knowledgeable than you are, will help the
project in the longer term.
Thanks. I don't mind rephrasing that four lines and amend the
log message of what I have in 'pu', if that is what you want.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Junio C Hamano @ 2012-12-13 23:42 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s0qK6yNiPe0ERDJWK-wfm3DdXZYwRzisoCPJ7PjsdkObQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> One of the review points were about this piece in the test:
>>
>> > +cmd=<<EOF
>> > +import bzrlib
>> > +bzrlib.initialize()
>> > +import bzrlib.plugin
>> > +bzrlib.plugin.load_plugins()
>> > +import bzrlib.plugins.fastimport
>> > +EOF
>> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>>
>> I cannot see how this could have ever worked.
>>
>> And I still don't see how your "would work just fine" can be true.
>
> As I have explained, all this code is the equivalent of python -c '',
> or rather, it's a no-op. It works in the sense that it doesn't break
> anything.
Aren't you ashamed of yourself after having said this?
> The purpose of the code is to check for the fastimport plug-in, but
> that plug-in is not used any more, it's vestigial code, it doesn't
> matter if the check works or not, as long as it doesn't fail.
If so, the final version that is suitable for merging would have
that unused code stripped away, no?
>> But it is totally a different matter to merge a crap with known
>> breakage that is one easy fix away from the get-go. Allowing that
>> means that all the times we spend on reviewing patches here go
>> wasted, discouraging reviewers.
>
> There is no breakage.
Unused code that burdens others to read through to make sure nothing
is broken is already broken from maintenance point of view.
Why are you wasting my time and everybody's bandwidth on this, when
you are very well capable of rerolling the series with removal and
style fixes in far shorter time?
^ permalink raw reply
* Re: Build fixes for another obscure Unix
From: David Michael @ 2012-12-13 22:30 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <871B6C10EBEFE342A772D1159D13208539FF9088@umechphg.easf.csd.disa.mil>
Hi,
On Thu, Dec 13, 2012 at 12:18 PM, Pyeron, Jason J CTR (US)
<jason.j.pyeron.ctr@mail.mil> wrote:
>> Would there be any interest in applying such individual compatibility
>> fixes for this system, even if a full port doesn't reach completion?
>
> What are the down sides? Can your changes be shown to not impact builds on other systems?
I've pushed a handful of small compatibility patches to GitHub[1]
which are enough to successfully compile the project. The default
values of the new variables should make them unnoticeable to other
systems.
Are there any concerns with this type of change? If they would be
acceptable, I can try sending the first four of those patches to the
list properly. (I expect the last two may be tweaked as I continue
working with the port.)
I do have a concern with strings.h, though. That file will be
included for most people who run ./configure, when it wasn't before.
Do you think it's worth making a more detailed test to see if
strcasecmp is still undefined after string.h is included, rather than
just testing for the header's existence?
Thanks.
David
[1] https://github.com/dm0-/git/commits
^ permalink raw reply
* Re: [PATCH] Fix sizeof usage in get_permutations
From: Felipe Contreras @ 2012-12-13 22:09 UTC (permalink / raw)
To: Joachim Schmitz; +Cc: git
In-Reply-To: <kacus2$mml$1@ger.gmane.org>
On Thu, Dec 13, 2012 at 10:13 AM, Joachim Schmitz
<jojo@schmitz-digital.de> wrote:
> Matthew Daley wrote:
>>
>> Currently it gets the size of an otherwise unrelated, unused variable
>> instead of the expected struct size.
>>
>> Signed-off-by: Matthew Daley <mattjd@gmail.com>
>> ---
>> builtin/pack-redundant.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
>> index f5c6afc..7544687 100644
>> --- a/builtin/pack-redundant.c
>> +++ b/builtin/pack-redundant.c
>> @@ -301,14 +301,14 @@ static void pll_free(struct pll *l)
>> */
>> static struct pll * get_permutations(struct pack_list *list, int n)
>> {
>> - struct pll *subset, *ret = NULL, *new_pll = NULL, *pll;
>> + struct pll *subset, *ret = NULL, *new_pll = NULL;
>>
>> if (list == NULL || pack_list_size(list) < n || n == 0)
>> return NULL;
>>
>> if (n == 1) {
>> while (list) {
>> - new_pll = xmalloc(sizeof(pll));
>> + new_pll = xmalloc(sizeof(struct pll));
>> new_pll->pl = NULL;
>> pack_list_insert(&new_pll->pl, list);
>> new_pll->next = ret;
>> @@ -321,7 +321,7 @@ static struct pll * get_permutations(struct
>> pack_list *list, int n) while (list->next) {
>> subset = get_permutations(list->next, n - 1);
>> while (subset) {
>> - new_pll = xmalloc(sizeof(pll));
>> + new_pll = xmalloc(sizeof(struct pll));
>
>
> Why not just
> new_pll = xmalloc(sizeof(*new_pll));
I prefer that one; it's Linux kernel style.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 4/7] remote-bzr: update working tree
From: Felipe Contreras @ 2012-12-13 22:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vtxs9vda3.fsf@alter.siamese.dyndns.org>
On Wed, Nov 28, 2012 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> contrib/remote-helpers/git-remote-bzr | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
>> index 2c05f35..5b89a05 100755
>> --- a/contrib/remote-helpers/git-remote-bzr
>> +++ b/contrib/remote-helpers/git-remote-bzr
>> @@ -571,6 +571,8 @@ def do_export(parser):
>> repo.generate_revision_history(revid, marks.get_tip('master'))
>> revno, revid = repo.last_revision_info()
>> peer.import_last_revision_info_and_tags(repo, revno, revid)
>> + wt = peer.bzrdir.open_workingtree()
>> + wt.update()
>> print "ok %s" % ref
>> print
>
> Shouldn't this be squashed as part of one of the earlier patches?
> The split between 1/7 (import first) and 2/7 (then support export)
> makes sense, but this one looks more like "oops, we forgot to touch
> the working tree while updating the history in the previous steps
> and here is a fix" to me.
Perhaps. It's not really clear if we should update the working tree at
all. A 'git push' doesn't update the working directory on the remote,
but a 'bzr push' does. I thought it was better to leave this
distinction clear, in case this becomes an issue later on.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-13 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwxhycii.fsf@alter.siamese.dyndns.org>
On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@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.
>
> One of us must be very confused. Perhaps you were looking at a
> wrong message (or I quoted a wrong one?).
>
> ... goes and double checks ...
>
> One of the review points were about this piece in the test:
>
> > +cmd=<<EOF
> > +import bzrlib
> > +bzrlib.initialize()
> > +import bzrlib.plugin
> > +bzrlib.plugin.load_plugins()
> > +import bzrlib.plugins.fastimport
> > +EOF
> > +if ! "$PYTHON_PATH" -c "$cmd"; then
>
> I cannot see how this could have ever worked.
>
> And I still don't see how your "would work just fine" can be true.
As I have explained, all this code is the equivalent of python -c '',
or rather, it's a no-op. It works in the sense that it doesn't break
anything.
The purpose of the code is to check for the fastimport plug-in, but
that plug-in is not used any more, it's vestigial code, it doesn't
matter if the check works or not, as long as it doesn't fail.
>>> but there may be others. It is the responsibility of a contributor to keep
>>> track of review comments others give to his or her patches and
>>> reroll them, so I do not recall every minor details, sorry.
>
> There may be others, but $gmane/210745 is also relevant, I think.
I'll comment on the patch, but I don't think it really prevents the merge.
>> There is nothing that prevents remote-bzr from being merged.
>
> What we merge may not be perfect. Bugs and misdesigns are often
> identified long after a topic is merged and it is perfectly normal
> we fix things up in-tree. There are even times where we say "OK, it
> is known to break if the user does something that pokes this and
> that obscure corners of this code, but the benefit of merging this
> 99% working code to help users that do not exercise the rare cases
> is greater than having them wait for getting the remaining 1% right,
> so let's merge it with known breakage documentation".
>
> But it is totally a different matter to merge a crap with known
> breakage that is one easy fix away from the get-go. Allowing that
> means that all the times we spend on reviewing patches here go
> wasted, discouraging reviewers.
There is no breakage.
> If you want others to take your patches with respect, please take
> reviewers' comments with the same respect you expect to be paid by
> others.
I don't need others to take my patches with respect, my patches are
not people, they don't have feelings.
That being said, I don't think I have disrespected any of your
comments. Yes, you are right that the above code is wrong and doesn't
do anything, what part of agreeing is disrespectful? But I don't agree
that it is a merge blocker. Disagreeing is not disrespecting.
This code was ready for 1.8.1, but it's not going to be there, so, I
don't see any hurry. As I said, I think the code is ready, and these
minor details can wait.
Cheers.
--
Felipe Contreras
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox