* rev-list behavior in mail notification when pushing multiple branches
From: Tim Niemueller @ 2009-08-17 16:13 UTC (permalink / raw)
To: git
Hi.
We are using git to manage a project which is separated in multiple
repositories. We use email notification for commits to the central
repositories, but the log output in these mails is not what we expect.
We use the contrib hook script from the git repository.
The main repository contains the trunk, in the real sense, meaning that
it is composed of infrastructure code shared among all repositories, not
in the svn sense. The other repositories contain the trunk master branch
as "trunk" branch. Their own master branch is based on trunk, but
extended by additional modules. We regularly merge from the main
repository into the other repositories trunk.
Consider the following situation:
--*---X-------Y master branch
/ /
A---B--C*---D trunk branch
master has been ultimately derived from trunk. At X trunk changes up to
C have been merged into the master. Then trunk evolves but master
remains unchanged. We merge again everything up to D from trunk. Now
assume that we haven't pushed since B/X and now push after D/Y. Then
there are two mails to be generated, one for master X..Y, effectively
containing log message Y (merged...), and one for trunk B..D, C* is any
number of intermediate commits to the trunk.
It happens to be that master has been pushed first. The email contains
the expected output in the log section. However, for the trunk branch
the output unexpectedly is empty, meaning that information about C* is
never send in any mail.
The reason is the following: in show_new_revisions "rev-parse --not" is
used to generate "stop" points for rev-list. The stop point issued for
the master branch is Y. This causes rev-list to stop immediately, since
this is the last commit in the chronological order.
What we would like to have is the that the master mail only states
"merged trunk changes" (as it is right now, if there were commits
between X and Y these should be included as well), while the trunk mail
issues all C* commits and D.
I have tried to come up with a way to filter out entries from the mails
that have been issued on "the other" branch(es) originally (although
they are now /after/ merging also part of the this branch's history).
Since gitk can show this there should be a way to figure it out, but my
git-fu isn't strong enough, yet. Can someone give advice on how to
achieve this, or how you solved the problem, if you did?
Thanks,
Tim
--
AllemaniACs RoboCup Team KBSG - Knowledge-Based Systems Group
========================================================================
http://robocup.rwth-aachen.de RWTH Aachen University
http://www.kbsg.rwth-aachen.de Ahornstrasse 55
http://www.fawkesrobotics.org D-52056 Aachen
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Nguyen Thai Ngoc Duy @ 2009-08-17 16:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jakub Narebski, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0908171712220.4991@intel-tinevez-2-302>
On Mon, Aug 17, 2009 at 10:19 PM, Johannes
Schindelin<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 17 Aug 2009, Nguyen Thai Ngoc Duy wrote:
>
>> On Mon, Aug 17, 2009 at 8:35 PM, Johannes
>> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>>
>> > The problem of course is that the other branch has an ancient version
>> > of that file (which should _not_ overwrite the current, modified
>> > version!), i.e. "git diff HEAD..other -- file" does not come empty.
>> >
>> > As 'file' is assume-unchanged, zinnnng, the file gets "updated".
>>
>> Then it is a bug. Assume-unchanged as in reading is good.
>> Assume-unchanged in writing sounds scary. Something like this should
>> fix it (not well tested though). It's on top of my series, but you can
>> adapt it to 'next' or 'master' easily.
>
> No.
>
> The purpose of 'assume-unchanged' is to tell Git that it has no business
> checking that the file is unchanged. It should _assume_ that it is
> unchanged. That's what this flag says.
>
> So do you agree that assume-changed is not quite similar enough to sparse
> to use the same bit?
If you define it that way, yes I agree.
>> > Another use case: documentation. I do not have that use case yet, but
>> > I know about people who do.
>>
>> Translators usually checkout one or two files (I am Vietnamese
>> Translation Coordinator of GNOME, but well... I check them all out. I
>> suppose "normal" translators would not want to do like I do.)
>
> Exactly.
>
> echo /Documentation/ > .git/info/sparse
>
> Remember: the documentation contributors are the least programming-savvy
> contributors of any project.
[wanted to make a joke here, but it seemed destructive, snipped]
>> > Specifying what you _want_ to have checked out is much more
>> > straight-forward here than the opposite.
>>
>> I think it depends on type of projects. For documentation projects, you
>> may want a few files. For software projects, usually you need everything
>> _except_ a few big directories. For WebKit, it's a bunch of test data
>> that I don't care about. Firmware in hardware-related projects or media
>> files in game projects fall in the same category. I don't have strong
>> opinion on this. Either include or exclude is fine to me.
>
> Okay, let me just ask: if you have a sparse checkout, what would you think
> I mean when I talk about the "sparse files"?
If I have to answer in 2 seconds, "sparse files" are files in working
directory. If I have more time, I tend to think that in "sparse
<something>", something should be a container, an area, therefore
"sparse files" do not make sense to me while "sparse
checkout/worktree" does. So, .git/info/sparse-checkout (with "in"
patterns)?
--
Duy
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Johannes Schindelin @ 2009-08-17 16:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vtz06xxao.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 17 Aug 2009, Junio C Hamano wrote:
> I'd say that assume-unchanged is a promise you make git that you won't
> change these paths, and in return to the promise git will give you
> faster response by not running lstat on them. Having changes in such
> paths is your problem and you deserve these chanegs to be lost. At
> least, that is the interpretation according to the original
> assume-unchanged semantics.
That's why I did not suggest using assume-unchanged (which the guy did
previously, and was burnt, deservedly, as you say).
However, my illustration of the scenario was only to one end, namely to
convince all of you that assume-changed != sparse.
And maybe to the end to explain that sparse checkout could help this guy.
Ciao,
Dscho
^ permalink raw reply
* Re: Linus' sha1 is much faster!
From: Linus Torvalds @ 2009-08-17 16:22 UTC (permalink / raw)
To: Steven Noonan
Cc: Giuseppe Scrivano, Pádraig Brady, Bug-coreutils,
Git Mailing List
In-Reply-To: <f488382f0908170844h649126efxb27f87d7b319961b@mail.gmail.com>
On Mon, 17 Aug 2009, Steven Noonan wrote:
>
> Interesting. I compared Linus' implementation to the public domain one
> by Steve Reid[1]
You _really_ need to talk about what kind of environment you have.
There are three major issues:
- Netburst vs non-netburst
- 32-bit vs 64-bit
- compiler version
Steve Reid's code looks great, but the way it is coded, gcc makes a mess
of it, which is exactly what my SHA1 tries to avoid.
[ In contrast, gcc does very well on just about _any_ straightforward
unrolled SHA1 C code if the target architecture is something like PPC or
ia64 that has enough registers to keep it all in registers.
I haven't really tested other compilers - a less aggressive compiler
would actually do _better_ on SHA1, because the problem with gcc is that
it turns the whole temporary 16-entry word array into register accesses,
and tries to do register allocation on that _array_.
That is wonderful for the above-mentioned PPC and IA64, but it makes gcc
create totally crazy code when there aren't enough registers, and then
gcc starts spilling randomly (ie it starts spilling a-e etc). This is
why the compiler and version matters so much. ]
> (average of 5 runs)
> Linus' sha1: 283MB/s
> Steve Reid's sha1: 305MB/s
So I get very different results:
# TIME[s] SPEED[MB/s]
Reid 2.742 222.6
linus 1.464 417
this is Intel Nehalem, but compiled for 32-bit mode (which is the more
challenging one because x86-32 only has 7 general-purpose registers), and
with gcc-4.4.0.
Linus
^ permalink raw reply
* Re: [PATCH 01/11] Fix build failure at VC because function declare use old style at regex.c
From: Johannes Schindelin @ 2009-08-17 16:26 UTC (permalink / raw)
To: Frank Li; +Cc: git, kusmabite, msysgit
In-Reply-To: <1250524872-5148-1-git-send-email-lznuaa@gmail.com>
Hi,
reading "X-Mailer: git-send-email 1.6.4.msysgit.0" gave me a buzz... well
done, Erik!
On Tue, 18 Aug 2009, Frank Li wrote:
> regerror declare function argument type after function define.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about
Avoid a K&R style function definition in regex.c
Microsoft Visual C++ does not understand K&R notation; use C89
style instead.
?
> diff --git a/compat/regex/regex.c b/compat/regex/regex.c
> index 5ea0075..5728de1 100644
> --- a/compat/regex/regex.c
> +++ b/compat/regex/regex.c
> @@ -4852,11 +4852,7 @@ regexec (preg, string, nmatch, pmatch, eflags)
> from either regcomp or regexec. We don't use PREG here. */
>
> size_t
> -regerror (errcode, preg, errbuf, errbuf_size)
> - int errcode;
> - const regex_t *preg;
> - char *errbuf;
> - size_t errbuf_size;
> +regerror (int errcode, const regex_t * preg, char * errbuf,size_t errbuf_size)
A cursory look over regex.c gives me the impression that
- it tries to stick to maximally 80 characters per line,
- there is no space after a * indicating a pointer,
- there are spaces after all commas,
- there are a lot more functions with K&R style function definitions than
just regerror().
Ciao,
Dscho
^ permalink raw reply
* [RFCv4 6/5] Fix the Makefile-generated path to the git_remote_cvs package in git-remote-cvs
From: Johan Herland @ 2009-08-17 16:27 UTC (permalink / raw)
To: git; +Cc: Johannes.Schindelin, barkalow, davvid, gitster
In-Reply-To: <1250480161-21933-1-git-send-email-johan@herland.net>
Junio discovered that the installed git-remote-cvs executable is unable
to find the installed git_remote_cvs Python package. This is due to the
'instlibdir' target of git_remote_cvs/Makefile being invoked with
insufficient arguments (prefix and DESTDIR). This patch fixes that.
Signed-off-by: Johan Herland <johan@herland.net>
---
Sorry for the inconvenience.
...Johan
Makefile | 2 +-
git_remote_cvs/Makefile | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index b2af678..b9a7f25 100644
--- a/Makefile
+++ b/Makefile
@@ -1479,7 +1479,7 @@ endif # NO_PERL
ifndef NO_PYTHON
$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
$(QUIET_GEN)$(RM) $@ $@+ && \
- INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_cvs -s --no-print-directory instlibdir` && \
+ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_cvs -s --no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' instlibdir` && \
sed -e '1{' \
-e ' s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
-e '}' \
diff --git a/git_remote_cvs/Makefile b/git_remote_cvs/Makefile
index 061c247..0d9eb31 100644
--- a/git_remote_cvs/Makefile
+++ b/git_remote_cvs/Makefile
@@ -28,7 +28,7 @@ install: $(pysetupfile)
$(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix)
instlibdir: $(pysetupfile)
- @echo "$(prefix)/$(PYLIBDIR)"
+ @echo "$(DESTDIR_SQ)$(prefix)/$(PYLIBDIR)"
clean:
$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) clean -a
--
1.6.4.313.g38b9.dirty
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply related
* Re: [PATCH 02/11] Fix declare variable at mid of function
From: Johannes Schindelin @ 2009-08-17 16:29 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250524872-5148-2-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Some compiler such as MSVC can't support declear variable at mid of funtion at c file.
Please wrap your commit messages after 76 characters.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
> ---
How about this instead?
Avoid declaration after instruction
Microsoft Visual C++ does not understand this C99 style.
?
The patch itself is good.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 03/11] Define SNPRINTF_SIZE_CORR 1 when use MSVC build git
From: Johannes Schindelin @ 2009-08-17 16:32 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250524872-5148-3-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> There are not NUL at vsnprintf verstion of MSVC when rearch max len.
> Define vsnprintf to _vsnprintf. vsnprintf have deprecated.
How about this instead?
Define SNPRINTF_SIZE_CORR=1 for Microsoft Visual C++
The Microsoft C runtime's vsnprintf function does not add NUL at
the end of the buffer.
Further, Microsoft deprecated vsnprintf in favor of _vsnprintf, so
add a #define to that end.
The patch is good, although I suspect that the definition of vsnprintf is
better handled in the precompiler options in .vcproj.
Ciao,
Dscho
^ permalink raw reply
* Re: "make quick-install-man" broke recently
From: Linus Torvalds @ 2009-08-17 16:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jacob Helwig, Kjetil Barvik, Randal L. Schwartz,
git@vger.kernel.org
In-Reply-To: <7vhbw72ap3.fsf@alter.siamese.dyndns.org>
On Sun, 16 Aug 2009, Junio C Hamano wrote:
>
> -/* "careful lstat()" */
> -extern int check_path(const char *path, int len, struct stat *st);
> -
> #define REFRESH_REALLY 0x0001 /* ignore_valid */
> #define REFRESH_UNMERGED 0x0002 /* allow unmerged */
> #define REFRESH_QUIET 0x0004 /* be quiet about it */
> diff --git a/entry.c b/entry.c
> index f276cf3..6813f8a 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -179,9 +179,18 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
> * This is like 'lstat()', except it refuses to follow symlinks
> * in the path.
> */
> -int check_path(const char *path, int len, struct stat *st)
> +static int check_path(const char *path, int len, struct stat *st,
> + const struct checkout *co)
> {
> - if (has_symlink_leading_path(path, len)) {
> + if (co->base_dir_len) {
> + const char *slash = path + len;
> + while (path < slash && *slash != '/')
> + slash--;
> + if (!has_dirs_only_path(path, slash-path, co->base_dir_len)) {
> + errno = ENOENT;
> + return -1;
> + }
> + } else if (has_symlink_leading_path(path, len)) {
Grr. Now 'check_path()' is no longer something generically useful.
Could you perhaps instead only change 'checkout_entry()' to do this hack,
and leave 'check_path()' as a generic replacement for "lstat()" that
doesn't follow symlinks?
Linus
^ permalink raw reply
* Re: [PATCH 02/11] Fix declare variable at mid of function
From: Reece Dunn @ 2009-08-17 16:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Frank Li, git, msysgit
In-Reply-To: <alpine.DEB.1.00.0908171827040.4991@intel-tinevez-2-302>
2009/8/17 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Tue, 18 Aug 2009, Frank Li wrote:
>
>> Some compiler such as MSVC can't support declear variable at mid of funtion at c file.
>
> Please wrap your commit messages after 76 characters.
>
>>
>> Signed-off-by: Frank Li <lznuaa@gmail.com>
>> ---
>
> How about this instead?
>
> Avoid declaration after instruction
>
> Microsoft Visual C++ does not understand this C99 style.
>
> ?
>
> The patch itself is good.
Shouldn't GCC be changed to use -std=c89 as well to pick up errors for
compilers that don't support c99 (like the Microsoft Visual C++ C
compiler)?
- Reece
^ permalink raw reply
* Re: [PATCH 04/11] Add _MSC_VER predefine macro to make same behaviors with __MINGW32__ Enable MSVC build. MSVC have the save behaviors with msysgit.
From: Johannes Schindelin @ 2009-08-17 16:38 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250524872-5148-4-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about
Test whether WIN32 is defined rather than __MINGW32__
The code which is conditional on MinGW32 is actually conditional
on Windows. So test WIN32 rather than __MINGW32__.
This does not break Cygwin builds, as WIN32 is undefined there.
Suggested by Dmitry Potapov
?
Of course, you have to edit your patch accordingly, then.
And yes, I just tested, WIN32 is indeed defined on MinGw32.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Junio C Hamano @ 2009-08-17 16:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Jakub Narebski, Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vtz06xxao.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Local changes in git do not belong to any particular branch. They belong
> to the work tree and the index. Hence you (1) can switch from branch A to
> branch B iff the branches do not have difference in the path with local
> changes, and (2) have to stash save, switch branches and then stash pop if
> you have local changes to paths that are different between branches you
> are switching between.
>
> How should assume-unchanged play with this philosophy?
>
> I'd say that assume-unchanged is a promise you make git that you won't
> change these paths, and in return to the promise git will give you faster
> response by not running lstat on them. Having changes in such paths is
> your problem and you deserve these chanegs to be lost. At least, that is
> the interpretation according to the original assume-unchanged semantics.
Having said that, we could (re)define assume-unchanged to mean "I may or
may not have changes to these paths, but I do not mean to commit them, so
do not show them as modified when I ask you for diff. But the changes are
precious nevertheless".
I think the writeout codepath pays attention to assume-unchanged bit
already for that reason (CE_MATCH_IGNORE_VALID is all about this issue).
So with that, how should assume-unchanged play with the "local changes
belong to the index and the work tree"?
- When adding to the index, the changes should be ignored;
- When checking out of the index? I.e. the user tells "git checkout
path" when path is marked as assume-unchanged. Such an explicit
request should probably lose the local changes in the work tree.
- When checking out of a commit? The same deal.
- When switching branches?
- If the branches do not touch assume-unchanged paths, we should keep
changes _and_ assume-unchanged bit. I do not know if that is what
the current code does.
- If the branches do touch assume-unchanged paths, what should happen?
We shouldn't blindly overwrite the local changes, so at least we
should change the code to error out if we do not already do so. But
then what? How does the user deal with this? Perhaps...
- Drop assume-unchanged temporarily;
- Stash save;
- Switch;
- Stash pop;
- Add assume-unchanged again.
???
Is such an updated (or "corrected") assume-unchanged any different from a
sparse checkout? After all, paths that are not to be checked out in a
sparse checkout are "pretend that the lack of these paths are illusion--they
are logically there. I do not intend to commit their removal, and I do not
want to lose the sparseness across branch switch".
There is one nit about this. If a path is outside the checkout area,
should it unconditionally stay outside the checkout area when you switch
branches? I may be interested in not checking out Documentation/
subdirectory and that may hold true for all _my_ branches, and it is a
sane thing not to complain "Oops, you actually removed Makefile in
Documentation/ in your work tree in reality, and you are switching to
another branch that has a different Makefile --- it is a delete-modify
conflict you need to resolve, and we won't let you switch branches" in
such a case.
But is that generally true in all "sparse checkout" settings?
It is unfortunate that this message raises more questions than it answers,
but I think a sparse checkout will have to answer them, whether it uses a
bit separate from assume-unchanged or it reuses the assume-unchanged bit.
^ permalink raw reply
* Re: sparse support in pu
From: James Pickens @ 2009-08-17 16:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Nguyen Thai Ngoc Duy, Johannes Sixt, skillzero, git
In-Reply-To: <alpine.DEB.1.00.0908171425410.4991@intel-tinevez-2-302>
On Mon, Aug 17, 2009, Johannes Schindelin<Johannes.Schindelin@gmx.de> wrote:
> The term 'phantom' is not specified at all. At least interested people on
> the mailing list know 'sparse'. But I agree that the naming is a major
> problem, hence my earlier (unanswered) call.
I don't particularly like 'phantom' either, but I haven't come up with any
good alternatives. 'sparse' seems fine to me, though it might make sense
to lengthen it to 'sparse-checkout'.
> However, I would find specifying what you do _not_ want in that file
> rather unintuitive, in the same leage as receive.denyNonFastForwards = no.
>
> If I want to have a sparse checkout, I know which files I _want_.
I agree; specifying which files you don't want seems backwards to me.
James
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Johannes Schindelin @ 2009-08-17 16:49 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525040-5868-1-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
> It will malloc buffer each time.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
Work around Microsoft Visual C++ not having va_copy()
In winansi.c, Git wants to know the length of the formatted string
so it can allocate enough space for it. But Microsoft Visual C++
does not have va_copy(), so we have to guess.
The problem is the guessing part:
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 9217c24..6091138 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -310,9 +314,13 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
> if (!console)
> goto abort;
>
> +#ifndef _MSC_VER
> va_copy(cp, list);
> len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
> va_end(cp);
> +#else
> + len= sizeof(small_buf) ;
> +#endif
small_buf only is 256 bytes. How do you want to make sure that the
subsequent vsnprintf() is not writing outside of the buffer?
Also, you still miss a space between "len" and "=".
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 06/11] Add miss git-compat-util.h at regex.c and fnmatch.c Add git-compat-util.h to enable build at MSVC environment
From: Johannes Schindelin @ 2009-08-17 16:51 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-1-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
Add missing git-compat-util.h to regex.c and fnmatch.c
This will be needed to compile with Microsoft Visual C++.
> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
> index 14feac7..5cbd49c 100644
> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -16,6 +16,10 @@
> write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA 02111-1307, USA. */
>
> +#ifdef _MSC_VER
> +#include "git-compat-util.h"
> +#endif
> +
Why not leave those #ifdef guards? Either they don't hurt, or they will
hurt Microsoft Visual C++, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Paolo Bonzini @ 2009-08-17 16:53 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit, Johannes.Schindelin
In-Reply-To: <1250525040-5868-1-git-send-email-lznuaa@gmail.com>
On 08/17/2009 06:04 PM, Frank Li wrote:
> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
> It will malloc buffer each time.
... but only a 257-byte buffer as dscho pointed out.
In many places that do not have va_copy, a simple assignment works. And
va_end is almost always a no-op. So what about
#ifndef va_copy
#define va_copy(dst, src) ((dst) = (src))
#endif
if it works on MSVC?
Paolo
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Reece Dunn @ 2009-08-17 16:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <4A898B27.3040507@gnu.org>
2009/8/17 Paolo Bonzini <bonzini@gnu.org>:
> On 08/17/2009 06:04 PM, Frank Li wrote:
>>
>> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
>> It will malloc buffer each time.
>
> ... but only a 257-byte buffer as dscho pointed out.
>
> In many places that do not have va_copy, a simple assignment works. And
> va_end is almost always a no-op. So what about
>
> #ifndef va_copy
> #define va_copy(dst, src) ((dst) = (src))
> #endif
>
> if it works on MSVC?
According to http://stackoverflow.com/questions/558223/vacopy-porting-to-visual-c
that should work.
- Reece
^ permalink raw reply
* Re: [PATCH 07/11] Add O_BINARY flag to open flag at mingw.c
From: Johannes Schindelin @ 2009-08-17 16:58 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-2-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Windows will convert CR\LF and union code at text mode.
> Git doesn't like this. Add O_BINARY flag to open function
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
mingw.c: Use the O_BINARY flag to open files
On Windows, non-text files must be opened using the O_BINARY flag.
MinGW does this for us automatically, but Microsoft Visual C++
does not.
Also, Johannes said that this would be a nice cleanup.
BTW what about fopen()?
Patch is obviously good.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 08/11] Place __stdcall to correct position.
From: Johannes Schindelin @ 2009-08-17 17:01 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-3-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> MSVC require __stdcall is between return value and function name.
> ALL Win32 API definition is as TYPE WINAPI function name
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about "... to the correct ..." and "MSVC requires _stdcall to be
between return value..." and "All Win32 API functions are declared with
the WINAPI attribute."?
> diff --git a/compat/mingw.c b/compat/mingw.c
> index d5fa0ed..0c9c793 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1146,7 +1146,7 @@ void mingw_open_html(const char *unixpath)
>
> int link(const char *oldpath, const char *newpath)
> {
> - typedef BOOL WINAPI (*T)(const char*, const char*, LPSECURITY_ATTRIBUTES);
> + typedef BOOL (WINAPI *T)(const char*, const char*, LPSECURITY_ATTRIBUTES);
An extra space slipped in, there.
> diff --git a/run-command.c b/run-command.c
> index df139da..423b506 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -295,12 +295,12 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> }
>
> #if defined(__MINGW32__) || defined(_MSC_VER)
> -static __stdcall unsigned run_thread(void *data)
> +static unsigned __stdcall run_thread(void *data)
> {
> struct async *async = data;
> return async->proc(async->fd_for_proc, async->data);
> }
> -#endif
> +#endif /* __MINGW32__ || _MSC_VER */
I do not think this is necessary. There are only 5 lines wrapped into
those #ifdef guards, the developer should be able to see that far.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Erik Faye-Lund @ 2009-08-17 17:02 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <4A898B27.3040507@gnu.org>
On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini<bonzini@gnu.org> wrote:
> #ifndef va_copy
> #define va_copy(dst, src) ((dst) = (src))
> #endif
Are you sure va_copy is always a preprocessor symbol? How about
#ifdef _MSC_VER
#define va_copy(dst, src) ((dst) = (src))
#endif
instead? It'd make me sleep slightly better at night, at least ;)
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: Linus' sha1 is much faster!
From: Nicolas Pitre @ 2009-08-17 17:06 UTC (permalink / raw)
To: George Spelvin; +Cc: bdonlan, johnflux, P, art.08.09, git, torvalds
In-Reply-To: <20090817072315.4314.qmail@science.horizon.com>
On Mon, 17 Aug 2009, George Spelvin wrote:
> If it helps anyone resolve license issues, here's a from-FIPS-180-2
> implementation that's placed in the public domain. That should be
> compatible with any license.
>
> It uses Linus's and Artur's performance ideas, and some of Linus' macro
> ideas (in the rotate implementation), but tries to be textually different.
> Is there anything recognizable that anyone cares to clam copyright to?
I don't think this trick of making source code textually different from
another work while still intimately mimicking the same structure entitles
you to any copyright (or non copyright) claims over that other work. I
certainly wouldn't bet any dime for this standing up in court.
Otherwise anyone could grab any copyrighted source code and perform a
bunch of search-and-replace ops on it, and maybe some code reordering
for good measure, to be able to claim own copyright on it. It is
probably much safer to simply ask the people involved to agree with your
relicensing. And so far I don't see anyone with a stake in this
fiercely wanting to stick to a particular license.
> It's not quite 100% finished, as I haven't benchmarked it against Linus's
> code yet, but it's functionally correct.
>
> It's also clean with -W -Wall -Wextra.
Not if you try with the unaligned put_be32() as the destination pointer
is marked const.
As to the actual result on ARM... Well, the assembly _looks_ much worse
than Linus' version. It uses a stack frame of 152 bytes instead of 64
bytes. The resulting binary is also 6868 bytes large compared to 6180
bytes. Surprisingly, the performance is not that bad (the reason for
the underlined "looks" above) albeit still a bit worse, like 5% slower.
I was expecting much worse than that.
One possible reason for the bad assembly is probably due to the fact
that gcc is not smart enough to propagate constant address offsets
across different pointer types. For example, my first version of
get_be32() was a macro that did this:
#define SHA_SRC(t) \
({ unsigned char *__d = (unsigned char *)&data[t]; \
(__d[0] << 24) | (__d[1] << 16) | (__d[2] << 8) | (__d[3] << 0); })
With such a construct, gcc would always allocate a register to hold __d
and then dereference that with an offset from 0 to 3. Whereas:
#define SHA_SRC(t) \
({ unsigned char *__d = (unsigned char *)data; \
(__d[(t)*4 + 0] << 24) | (__d[(t)*4 + 1] << 16) | \
(__d[(t)*4 + 2] << 8) | (__d[(t)*4 + 3] << 0); })
does produce optimal assembly as only the register holding the data
pointer is dereferenced with the absolute byte offset. I suspect your
usage of inline functions has the same effect as the first SHA_SRC
definition above.
Also, wrt skipping the last 3 write back to the 16 word array... For
all the (limited) attempts I've made so far to do that, it always ended
up making things worse. I've yet to investigate why though.
Nicolas
^ permalink raw reply
* Re: "make quick-install-man" broke recently
From: Junio C Hamano @ 2009-08-17 17:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Jacob Helwig, Kjetil Barvik, Randal L. Schwartz,
git@vger.kernel.org
In-Reply-To: <alpine.LFD.2.01.0908170932390.3162@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Grr. Now 'check_path()' is no longer something generically useful.
>
> Could you perhaps instead only change 'checkout_entry()' to do this hack,
> and leave 'check_path()' as a generic replacement for "lstat()" that
> doesn't follow symlinks?
Given that non-empty base_dir is only used for "checkout-index --prefix",
iow, the "path" internally used by git and fed to our symlink.c cache are
supposed to be always relative to the work tree, I think that may be a
good thing to do in the short-term.
But only if we won't add any more like "checkout-index --prefix". If you
want to implement "git checkout --prefix=over-there/" and if you want to
call check_path() directly (iow not as a part of callchain from
checkout_entry()) while doing so, for example, you would regret keeping
the check_path() function unaware of base_dir, as you would reintroduce
the same bug.
I thought about getting rid of base_dir from struct checkout by running
create_directories() in checkout-index and chdir(2) there, because this is
a very special case codepath anyway. I actually haven't tried it, but it
probably will have bad interactions with the way we find $GIT_DIR.
^ permalink raw reply
* Re: [PATCH 09/11] Add MSVC porting header files.
From: Johannes Schindelin @ 2009-08-17 17:09 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-4-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Add unix head file, dirent.h, unistd.h and time.h
These are copied from somewhere. From where? What is the license?
> Add MSVC special porting head file msvc.h and msvc.c.
This is added by you. Logically, that should be a separate patch.
> diff --git a/compat/msvc.h b/compat/msvc.h
> new file mode 100644
> index 0000000..6071565
> --- /dev/null
> +++ b/compat/msvc.h
> @@ -0,0 +1,95 @@
> +#ifndef __MSVC__HEAD
> +#define __MSVC__HEAD
> +
> +#define WINVER 0x0500
> +#define _WIN32_WINNT 0x0500
> +#define _WIN32_WINDOWS 0x0410
> +#define _WIN32_IE 0x0700
> +#define NTDDI_VERSION NTDDI_WIN2KSP1
> +#include <winsock2.h>
> +
> +/*Configuration*/
> +
> +#define NO_PREAD
> +#define NO_OPENSSL
> +#define NO_LIBGEN_H
> +#define NO_SYMLINK_HEAD
> +#define NO_IPV6
> +#define NO_SETENV
> +#define NO_UNSETENV
> +#define NO_STRCASESTR
> +#define NO_STRLCPY
> +#define NO_MEMMEM
> +#define NO_C99_FORMAT
> +#define NO_STRTOUMAX
> +#define NO_MKDTEMP
> +#define NO_MKSTEMPS
> +
> +#define RUNTIME_PREFIX
> +#define NO_ST_BLOCKS_IN_STRUCT_STAT
> +#define NO_NSEC
> +#define USE_WIN32_MMAP
> +#define USE_NED_ALLOCATOR
> +
> +#define NO_REGEX
> +
> +#define NO_SYS_SELECT_H
> +#define NO_PTHEADS
> +#define HAVE_STRING_H 1
> +#define STDC_HEADERS
> +#define NO_ICONV
These would normally be defined in the Makefile. You might want to state
that in a comment.
Or maybe move the definitions (along with vsnprintf) to the .vcproj file,
which is the logical pendant of the Makefile?
> +#define inline __inline
> +#define __inline__ __inline
These definitions are unrelated to the surrounding ones; please move them
elsewhere.
> +
> +#define SNPRINTF_RETURNS_BOGUS
> +
> +#define SHA1_HEADER "mozilla-sha1\\sha1.h"
> +
> +#define ETC_GITCONFIG "%HOME%"
> +
> +#define NO_PTHREADS
> +#define NO_CURL
> +
> +
> +#define NO_STRTOUMAX
> +#define REGEX_MALLOC
> +
> +
> +#define GIT_EXEC_PATH "bin"
> +#define GIT_VERSION "1.6"
> +#define BINDIR "bin"
> +#define PREFIX "."
> +#define GIT_MAN_PATH "man"
> +#define GIT_INFO_PATH "info"
> +#define GIT_HTML_PATH "html"
> +#define DEFAULT_GIT_TEMPLATE_DIR "templates"
> +
> +#define NO_STRLCPY
> +#define NO_UNSETENV
> +#define NO_SETENV
Would these NO_ definitions not _love_ to be close to their siblings?
What is the reason for those empty lines? Their placement and amount look
rather arbitrary to me.
> +#define strdup _strdup
> +#define read _read
> +#define close _close
> +#define dup _dup
> +#define dup2 _dup2
> +#define strncasecmp _strnicmp
> +#define strtoull _strtoui64
vsnprintf could go right here.
> +#define __attribute__(x)
The two inline definitions could go right here.
> +static __inline int strcasecmp (const char *s1, const char *s2)
> +{
> + int size1=strlen(s1);
> + int sisz2=strlen(s2);
> +
> + return _strnicmp(s1,s2,sisz2>size1?sisz2:size1);
> +}
> +
> +#include "compat/mingw.h"
> +#undef ERROR
> +#undef stat
> +#define stat(x,y) mingw_lstat
> +#define stat _stat64
> +#endif
Looks much nicer now, thanks!
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 10/11] Add MSVC Project file
From: Johannes Schindelin @ 2009-08-17 17:11 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-5-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Add libgit.vcproj to build common library.
> Add git.vcproj to build git program.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
The commit subject should read "... files", as you add two files, not one.
I hope that somebody else is going to review this patch...
And don't you lack .vcproj files for the non-builtins?
Ciao,
Dscho
^ permalink raw reply
* Re: sparse support in pu
From: Johannes Schindelin @ 2009-08-17 17:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Peter Harris, Nguyen Thai Ngoc Duy, Johannes Sixt, skillzero, git
In-Reply-To: <7v7hx2xw6p.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 17 Aug 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> That's funny. I have a git tree that would benefit from sparse checkout.
> >> I know which path I _don't_ want. Specifying all the paths I want would
> >> be a rather longer (and more error-prone) list. I suspect it would be
> >> best to support both.
> >
> > Yes, I agree, but the common case is for people to know what they are
> > working on, right?
>
> I either may be working on Documentation/ (I know I do not care about
> everthing else so I can afford to list Makefile and Documentation/ in
> the "interesting" list), or I either may be working on code (I do not
> know how many code directories there are, and do not care to list them,
> but I know I do not need Documentation/ and contrib/). You need both
> ways.
>
> And with .gitignore syntax you can have both.
I was fully aware of this. It was the only reason I proposed the
.gitignore syntax.
My point was that the direction should be dictated by convenience to those
who need the feature most.
And maybe by asking yourself the question "what would I understand by
'sparse files'?"
Ciao,
Dscho
^ 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