* Re: Shell script cleanups/style changes?
From: Junio C Hamano @ 2007-08-02 21:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: David Kastrup, git
In-Reply-To: <fcaeb9bf0708021356v57b29a70yb69a2fa000bd5b55@mail.gmail.com>
"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> On 8/2/07, Junio C Hamano <gitster@pobox.com> wrote:
>> Non POSIX substitions such as ${parameter/pattern/string} and
>> ${parameter:offset} are not to be used. We do not want to
>> depend on bash.
>
> There is in a test (t5300-pack-objects.sh) but I guess the
> restrictions do not apply on tests.
That would have been an earlier mistake. Keeping tests portable
is important, perhaps it might be of lessor importance but
still.
^ permalink raw reply
* Re: [PATCH] Fix set_work_tree on cygwin
From: Junio C Hamano @ 2007-08-02 21:14 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0708022204170.14781@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Thu, 2 Aug 2007, Alex Riesen wrote:
>
>> Johannes Schindelin, Thu, Aug 02, 2007 17:38:37 +0200:
>>
>> > On Thu, 2 Aug 2007, Alex Riesen wrote:
>> >
>> > >@@ -209,7 +209,8 @@ const char *set_work_tree(const char *dir)
>> > > len = strlen(dir);
>> > > if (len > postfix_len && !strcmp(dir + len - postfix_len,
>> > > "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
>> > >- strncpy(dir_buffer, dir, len - postfix_len);
>> > >+ strncpy(dir_buffer, dir, len - postfix_len);
>> > >+ dir_buffer[len - postfix_len] = '\0';
>> > >
>> > > /* are we inside the default work tree? */
>> > > rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
>> >
>> > Darn, darn, darn. strncpy does _not_ NUL terminate. I keep forgetting
>> > that.
>> >
>> > Better use strlcpy()?
>>
>> Of course, but it just should not be needed at all: static supposed to
>> be zeroed.
>
> Certainly. But reality outweighs theory, and so I Ack either your patch
> or replacing it by strlcpy().
Static is supposed to be zeroed and also is supposed to retain
the value from the previous call. I am guessing from the change
to make "rel" to non-static that this function is called twice
perhaps?
^ permalink raw reply
* Re: Shell script cleanups/style changes?
From: Junio C Hamano @ 2007-08-02 21:21 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <85odhpzmbo.fsf@lola.goethe.zz>
David Kastrup <dak@gnu.org> writes:
> Sure. What about the git-rebase line using $(($end - $msgnum)) ?
> That's even more risque than ##.
Is it really risque? I do not think we have heard trouble with
the arith expansion from anybody. A few mistakes in the past
made that said things like:
$((end - 1)) ;# wrong... say "$end" if you mean variable
$((cd ...; pwd)) ;# wrong... say $( (...)) if command substitution
# that involves subshell
but I think we fixed them.
> Understood. But using ${...#...} and ${...:+...} does not exactly
> seem to be news in the git code base. Even though we have the claim
> that Solaris' sh won't deal with the former.
I do not think we have trouble with ${parameter#word}. Much
less with ${parameter+word}; it has been in /bin/sh forever.
^ permalink raw reply
* Git on MSys (or how to make it easy for Windows users to compile git)
From: Johannes Schindelin @ 2007-08-02 21:23 UTC (permalink / raw)
To: git
Hi,
I finally broke down, waiting for the sourceforge project to get granted.
In the meantime, I registered a project at
http://code.google.com/p/msysgit/
(WARNING: temporary only!)
Would you believe that Google code has a restriction to 20MB per file, and
100MB in total, and you cannot remove files? The same Google that gives
you 1TB mail space and counting? Yes, it is ludicrous.
Anyway, you can get a complete Development environment in 3 files (because
one would be too large), and... oh well, just read what is written on the
website if you're really interested.
The plan is to move to Source forget ;-) when they finally approve the
project, or stay with Google, should they decide to lift the quota a bit.
Dmitry already reported a buglet preventing gcc to run without changes on
Vista, his theory is that the infamous access() function is at fault. So
if you are unlucky enough to be stuck with Vista, just copy
msysGit/mingw/libexec/gcc/mingw32/3.4.2/cc1.exe to msysGit/mingw/bin/ and
you're set.
Ciao,
Dscho
^ permalink raw reply
* Re: Shell script cleanups/style changes?
From: Junio C Hamano @ 2007-08-02 21:29 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <7vsl71tyyq.fsf@assigned-by-dhcp.cox.net>
You might find this thread amusing.
http://thread.gmane.org/gmane.comp.version-control.git/7116/focus=7136
Historically, I have even avoided accepting ${var#word}, ${var%word},
and arithmetic expansions.
I would still reject shell arrays.
^ permalink raw reply
* Re: [PATCH] Fix set_work_tree on cygwin
From: Johannes Schindelin @ 2007-08-02 21:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <7vwswdtz98.fsf@assigned-by-dhcp.cox.net>
Hi,
On Thu, 2 Aug 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Hi,
> >
> > On Thu, 2 Aug 2007, Alex Riesen wrote:
> >
> >> Johannes Schindelin, Thu, Aug 02, 2007 17:38:37 +0200:
> >>
> >> > On Thu, 2 Aug 2007, Alex Riesen wrote:
> >> >
> >> > >@@ -209,7 +209,8 @@ const char *set_work_tree(const char *dir)
> >> > > len = strlen(dir);
> >> > > if (len > postfix_len && !strcmp(dir + len - postfix_len,
> >> > > "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
> >> > >- strncpy(dir_buffer, dir, len - postfix_len);
> >> > >+ strncpy(dir_buffer, dir, len - postfix_len);
> >> > >+ dir_buffer[len - postfix_len] = '\0';
> >> > >
> >> > > /* are we inside the default work tree? */
> >> > > rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
> >> >
> >> > Darn, darn, darn. strncpy does _not_ NUL terminate. I keep forgetting
> >> > that.
> >> >
> >> > Better use strlcpy()?
> >>
> >> Of course, but it just should not be needed at all: static supposed to
> >> be zeroed.
> >
> > Certainly. But reality outweighs theory, and so I Ack either your patch
> > or replacing it by strlcpy().
>
> Static is supposed to be zeroed and also is supposed to retain
> the value from the previous call. I am guessing from the change
> to make "rel" to non-static that this function is called twice
> perhaps?
Apparently (but I would feel safer with strlcpy() anyway). git-read-tree
is the first and only offender which comes up in the test suite:
-- snipsnap --
[PATCH] read-tree: remove unnecessary call to setup_git_directory()
read-tree is already marked with RUN_SETUP in git.c, so there is
no need to call setup_git_directory() a second time.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin-read-tree.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 41f8110..a3b17a3 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -97,7 +97,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
memset(&opts, 0, sizeof(opts));
opts.head_idx = -1;
- setup_git_directory();
git_config(git_default_config);
newfd = hold_locked_index(&lock_file, 1);
--
1.5.3.rc3.121.g7f37
^ permalink raw reply related
* Re: Shell script cleanups/style changes?
From: Robert Schiele @ 2007-08-02 21:41 UTC (permalink / raw)
To: David Kastrup, Junio C Hamano; +Cc: git
In-Reply-To: <85odhpzmbo.fsf@lola.goethe.zz>
[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]
On Thu, Aug 02, 2007 at 10:57:31PM +0200, David Kastrup wrote:
> Most of the "new tricks" I try on bash, dash and ash.
Well, those are not really the most challenging one. Thus you should either
test on more or just believe those people that have other shells that it does
not work.
> I am confused now: a different poster adamantly stated that /bin/sh on
> Solaris did not support those constructs, and that every functionality
> of git was working fine for him.
No, you should read the mails you are refering to. I said that the most
important stuff does work. Apparently this did not yet hurt me on the
platform. Thus we have to decide whether we want some textbook example code
and thus break this platform completely or whether we want to fix the issues
you have listed and thus have a more portable application.
> Sure. What about the git-rebase line using $(($end - $msgnum)) ?
Bad on Solaris:
$ uname -a
SunOS solaris10-x64 5.10 Generic i86pc i386 i86pc
$ end=1
$ msgnum=5
$ echo $(($end - $msgnum))
syntax error: `(' unexpected
$
> Too bad: this should mean that $EDITOR can get called from C... I've
> been glad to see that so far this could be avoided.
Why is it bad to call the editor from C?
On Thu, Aug 02, 2007 at 02:21:01PM -0700, Junio C Hamano wrote:
> David Kastrup <dak@gnu.org> writes:
>
> > Sure. What about the git-rebase line using $(($end - $msgnum)) ?
> > That's even more risque than ##.
>
> Is it really risque? I do not think we have heard trouble with
> the arith expansion from anybody. A few mistakes in the past
See above.
> I do not think we have trouble with ${parameter#word}. Much
$ uname -a
SunOS solaris10-x64 5.10 Generic i86pc i386 i86pc
$ parameter=bla
$ echo ${parameter#word}
bad substitution
$
> less with ${parameter+word}; it has been in /bin/sh forever.
That one is ok for Solaris.
Robert
--
Robert Schiele
Dipl.-Wirtsch.informatiker mailto:rschiele@gmail.com
"Quidquid latine dictum sit, altum sonatur."
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Shell script cleanups/style changes?
From: David Kastrup @ 2007-08-02 21:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl71tyyq.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> writes:
> David Kastrup <dak@gnu.org> writes:
>
>> Understood. But using ${...#...} and ${...:+...} does not exactly
>> seem to be news in the git code base. Even though we have the
>> claim that Solaris' sh won't deal with the former.
>
> I do not think we have trouble with ${parameter#word}. Much less
> with ${parameter+word}; it has been in /bin/sh forever.
Basically this should mean that the proposed cleanups (apart from a
forgotten shift I had to add) are tenable.
Given that another poster claimed that Solaris /bin/sh does not
support ${parameter#word}, making the suggested changes to git-commit
might actually be a good idea: ${parameter#word} is used in half a
dozen other (likely less used) utilities in various other places. If
this is an overlooked regression, we want to make it non-overlookable
while we are still in testing, and git-commit would appear to be the
perfect candidate for that...
Depending on the feedback, we can either replace _all_ uses
everywhere, or accept it for good.
While I would think it perfectly understandable if you wanted to avoid
making an infamous "breaks all of Solaris release", _if_ ${...#...}
would indeed be fishy (and I somewhat doubt it), we are already there.
I have this cleaned-up version of git-commit.sh on a computer I can't
access right now. I'll post the patch tomorrow. Whether you want to
apply it to git.git remains at your discretion. I would, however,
strongly urge Solaris and potentially other POSIXly impaired users to
aplly and test this patch: if it breaks (and it will do so pretty
obviously, pretty much being unable to parse any option), then this is
_quite_ alarming with regard to existing uses of ${...#...} and would
need to get addressed _very_ soon.
Frankly, I doubt that this would have escaped notice so far, however.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] Fix set_work_tree on cygwin
From: Junio C Hamano @ 2007-08-02 21:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0708022230070.14781@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Static is supposed to be zeroed and also is supposed to retain
>> the value from the previous call. I am guessing from the change
>> to make "rel" to non-static that this function is called twice
>> perhaps?
>
> Apparently (but I would feel safer with strlcpy() anyway)...
Yup, send an appliable "final" version, somebody please?
> ... git-read-tree
> is the first and only offender which comes up in the test suite:
It is unclear.
Is this an optimization, or enforcing the new world order? IOW,
is it now banned to call setup twice?
^ permalink raw reply
* Re: [PATCH] Handle the errors from chdir in set_work_tree
From: Junio C Hamano @ 2007-08-02 21:58 UTC (permalink / raw)
To: Alex Riesen; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <81b0412b0708020827p174515b7tc05fefde77f7d7c4@mail.gmail.com>
"Alex Riesen" <raa.lkml@gmail.com> writes:
> These I haven't seen yet. Wouldn't like such a surprise though.
> ...
> @@ -220,8 +220,10 @@ const char *set_work_tree(const char *dir)
> if (!is_absolute_path(dir))
> set_git_dir(make_absolute_path(dir));
> dir = dir_buffer;
> - chdir(dir);
> - strcat(rel, "/");
> + if (chdir(dir))
> + rel = NULL;
> + else
> + strcat(rel, "/");
> inside_git_dir = 0;
> } else {
> rel = NULL;
Shouldn't it die() instead, though?
Consolidating two of your patches, would this be Ok?
-- >8 --
Fix work-tree related breakages
In set_work_tree(), variable rel needs to be reinitialized to
NULL on every call (it should not be static).
Make sure the incoming dir variable is not too long before
copying to the temporary buffer, and make sure chdir to the
resulting directory succeeds.
---
setup.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/setup.c b/setup.c
index 3653092..4945eb3 100644
--- a/setup.c
+++ b/setup.c
@@ -201,26 +201,32 @@ int is_inside_work_tree(void)
*/
const char *set_work_tree(const char *dir)
{
- char dir_buffer[PATH_MAX];
- static char buffer[PATH_MAX + 1], *rel = NULL;
- int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
+ char dir_buffer[PATH_MAX], *rel = NULL;
+ static char buffer[PATH_MAX + 1];
+ int len, suffix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
/* strip the variable 'dir' of the postfix "/.git" if it has it */
len = strlen(dir);
- if (len > postfix_len && !strcmp(dir + len - postfix_len,
- "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
- strncpy(dir_buffer, dir, len - postfix_len);
+ if (len > suffix_len &&
+ !strcmp(dir + len - suffix_len, "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
+ if ((len - suffix_len) >= sizeof(dir_buffer))
+ die("directory name too long");
+ memcpy(dir_buffer, dir, len - suffix_len);
+ dir_buffer[len - suffix_len] = '\0';
/* are we inside the default work tree? */
rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
}
+
/* if rel is set, the cwd is _not_ the current working tree */
if (rel && *rel) {
if (!is_absolute_path(dir))
set_git_dir(make_absolute_path(dir));
dir = dir_buffer;
- chdir(dir);
- strcat(rel, "/");
+ if (chdir(dir))
+ die("cannot chdir to %s: %s", dir, strerror(errno));
+ else
+ strcat(rel, "/");
inside_git_dir = 0;
} else {
rel = NULL;
^ permalink raw reply related
* Re: Shell script cleanups/style changes?
From: David Kastrup @ 2007-08-02 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodhptyk3.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> writes:
> You might find this thread amusing.
>
> http://thread.gmane.org/gmane.comp.version-control.git/7116/focus=7136
>
> Historically, I have even avoided accepting ${var#word}, ${var%word},
> and arithmetic expansions.
I learnt Unix with a Banaham/Rutter primer in the early eighties. I
got hit so often by the "this is now supposed to work in Bourne
shells?" surprise it wasn't funny.
The first time I saw "for ((i=0; i<$NR; i++)) ..." I thought the
author had been smoking too much C and got things confused. It still
creeps me out. I am more comfortable doing arithmetic with dc rather
than sh.
Employing the existing globbing machinery for # and %, on the other
hand, seems quite bournesque (still-bourne sounds so ugly) to me. And
it is certainly quite more readable than the regexp/expr stuff.
If it works.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* [PATCH] Fix unterminated string copy in set_work_tree
From: Alex Riesen @ 2007-08-02 22:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0708022230070.14781@racer.site>
Use strlcpy which zero-terminates the output string
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Johannes Schindelin, Thu, Aug 02, 2007 23:36:37 +0200:
> On Thu, 2 Aug 2007, Junio C Hamano wrote:
> >
> > Static is supposed to be zeroed and also is supposed to retain
> > the value from the previous call. I am guessing from the change
> > to make "rel" to non-static that this function is called twice
> > perhaps?
Actually, I was very confused. When I wrote about cygwin problems,
I actually debugged it for dir_buffer, real stack-based variable,
which of course is not zero-initialized. For an unknown reason I
confused the variable with buffer, which is static. "rel" should
be left of this particular discussion (it just does not matter whether
it is static or not in this context).
So the fix is a real fix for real problem which just happens to be
invisible on our linux systems.
> Apparently (but I would feel safer with strlcpy() anyway). git-read-tree
> is the first and only offender which comes up in the test suite:
Yes, I feel so too, so here it is.
setup.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/setup.c b/setup.c
index 3653092..27d585c 100644
--- a/setup.c
+++ b/setup.c
@@ -209,7 +209,7 @@ const char *set_work_tree(const char *dir)
len = strlen(dir);
if (len > postfix_len && !strcmp(dir + len - postfix_len,
"/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
- strncpy(dir_buffer, dir, len - postfix_len);
+ strlcpy(dir_buffer, dir, len - postfix_len + 1);
/* are we inside the default work tree? */
rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
--
1.5.3.rc3.139.ga57724
^ permalink raw reply related
* [PATCH] Allow setup_work_tree() to be called several times
From: Johannes Schindelin @ 2007-08-02 22:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <7vk5sdtxx0.fsf@assigned-by-dhcp.cox.net>
setup_work_tree() used to rely on a static buffer being initialized to all
zeroes. While at it, unstatify a pointer.
Noticed by Alex Riesen.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Thu, 2 Aug 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> Static is supposed to be zeroed and also is supposed to retain
> >> the value from the previous call. I am guessing from the change
> >> to make "rel" to non-static that this function is called twice
> >> perhaps?
> >
> > Apparently (but I would feel safer with strlcpy() anyway)...
>
> Yup, send an appliable "final" version, somebody please?
Here you are.
> > ... git-read-tree
> > is the first and only offender which comes up in the test suite:
>
> It is unclear.
>
> Is this an optimization, or enforcing the new world order? IOW,
> is it now banned to call setup twice?
It is purely an optimization, because we allow calling setup twice
with this patch... but we do not recommend it, as it is
unnecessary.
setup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/setup.c b/setup.c
index 3653092..16745f9 100644
--- a/setup.c
+++ b/setup.c
@@ -201,15 +201,15 @@ int is_inside_work_tree(void)
*/
const char *set_work_tree(const char *dir)
{
- char dir_buffer[PATH_MAX];
- static char buffer[PATH_MAX + 1], *rel = NULL;
+ char dir_buffer[PATH_MAX], *rel = NULL;
+ static char buffer[PATH_MAX + 1];
int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
/* strip the variable 'dir' of the postfix "/.git" if it has it */
len = strlen(dir);
if (len > postfix_len && !strcmp(dir + len - postfix_len,
"/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
- strncpy(dir_buffer, dir, len - postfix_len);
+ strlcpy(dir_buffer, dir, len - postfix_len + 1);
/* are we inside the default work tree? */
rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
--
1.5.3.rc3.121.g7f37
^ permalink raw reply related
* Re: [PATCH] Handle the errors from chdir in set_work_tree
From: Johannes Schindelin @ 2007-08-02 22:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <7v8x8ttx7y.fsf@assigned-by-dhcp.cox.net>
Hi,
On Thu, 2 Aug 2007, Junio C Hamano wrote:
> Fix work-tree related breakages
>
> In set_work_tree(), variable rel needs to be reinitialized to
> NULL on every call (it should not be static).
>
> Make sure the incoming dir variable is not too long before
> copying to the temporary buffer, and make sure chdir to the
> resulting directory succeeds.
ACK.
(Forget my patch, please)
Ciao,
Dscho
^ permalink raw reply
* Re: cvs2svn conversion directly to git ready for experimentation
From: Steffen Prohaska @ 2007-08-02 22:02 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: Michael Haggerty, Git Mailing List
In-Reply-To: <46B2309E.3060804@fs.ei.tum.de>
[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]
Simon,
On Aug 2, 2007, at 9:29 PM, Simon 'corecode' Schubert wrote:
> Steffen Prohaska wrote:
>> I remember that togit reported a broken pipe. My feeling was
>> that git-fastimport aborted, which may be reason why tohg
>> worked better. I didn't try to understand more details. I never
>> read ruby code before and it was already a challenge for me to
>> get everything up and running (rcs, rbtree).
>
> yah, that pretty much tells me it is shawn's bug :) but without
> more details, it is very hard to diagnose.
I tried again. Interestingly now togit works but tohg still fails.
togit starts with reporting
fatal: Not a valid object name
as the first line. But besides that it seems to work fine. What
concerns me a bit is that the last line togit reports is
committing set 18100/18173
I'd expect it should report 18173/18173.
The rest are git-fast-import statistics.
BTW, togit creates much more complex branching patterns than cvs2svn
does. The attached file branching.png displays a small view of a
branching pattern that extends downwards over a couple of screens.
I checked the cvs2svn history again. It doesn't contain anything
of similar complexity.
> tohg should tell you which rcs revs are the offenders. be sure to
> use a recent fromcvs however.
tohg fails (on the same repo that togit imported) with the
following error
Traceback (most recent call last):
File "./tohg.py", line 102, in <module>
destrepo.dispatch()
File "./tohg.py", line 98, in dispatch
func(*l[1:])
File "./tohg.py", line 78, in cmd_commit
extra = {'branch': branch})
File "/sw/lib/python2.5/site-packages/mercurial/localrepo.py",
line 736, in commit
mn = self.manifest.add(m1, tr, linkrev, c1[0], c2[0], (new,
remove))
File "/sw/lib/python2.5/site-packages/mercurial/manifest.py", line
191, in add
_("failed to remove %s from manifest") % f)
AssertionError: failed to remove X/Y.cpp from manifest
transaction abort!
rollback completed
./tohg.rb:200:in `readline': End of file reached while handling set
[core/X/Y.cpp,v:1.19,core/X/Z.cpp,v:1.22,core/X/Attic/W,v:1.12]
(EOFError)
from ./tohg.rb:200:in `_commit'
from ./tohg.rb:154:in `commit'
from ./fromcvs.rb:894:in `commit'
from ./fromcvs.rb:965:in `commit_sets'
from ./tohg.rb:228
The versions I used are listed below. I adjusted tohg a bit to use
python 2.5
installed by fink. I'm working on Mac OS X.
$ cd fromcvs
$ hg tip
changeset: 103:cccdab84e9e5
tag: tip
user: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
date: Mon Jul 16 23:49:52 2007 +0200
summary: Add error handling on committing sets.
$ hg diff
diff -r cccdab84e9e5 tohg.rb
--- a/tohg.rb Mon Jul 16 23:49:52 2007 +0200
+++ b/tohg.rb Fri Jul 20 17:06:30 2007 +0200
@@ -60,7 +60,7 @@ class HGDestRepo
@status = status
@outs, @ins = \
- Open2.popen2('python', File.join(File.dirname($0), 'tohg.py'),
hgroot)
+ Open2.popen2('python2.5', File.join(File.dirname($0),
'tohg.py'), hgroot)
@last_date = Time.at(@ins.readline.strip.to_i)
@branches = {}
while l = @ins.readline do
$ cd rcsparse
$ hg tip
changeset: 37:e871e108f2e4
tag: tip
user: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
date: Sun Feb 18 15:46:29 2007 +0100
summary: Return revision date in GMT, like RCS/CVS uses everywhere.
rbtree-0.2.0.tar.gz
ruby 1.8.2 (2004-12-25) [universal-darwin8.0]
$ cd git
$ git describe master
v1.5.3-rc3-120-g68d4229
$ hg --version
Mercurial Distributed SCM (version 0.9.3)
Copyright (C) 2005, 2006 Matt Mackall <mpm@selenic.com>
This is free software; see the source for copying conditions. There
is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.
$ /sw/bin/python2.5 --version
Python 2.5.1
Hope this helps.
Steffen
[-- Attachment #2.1: branching.png --]
[-- Type: application/applefile, Size: 73 bytes --]
[-- Attachment #2.2: branching.png --]
[-- Type: image/png, Size: 17562 bytes --]
[-- Attachment #3: Type: text/plain, Size: 1 bytes --]
^ permalink raw reply
* Re: Shell script cleanups/style changes?
From: David Kastrup @ 2007-08-02 22:14 UTC (permalink / raw)
To: Robert Schiele; +Cc: Junio C Hamano, git
In-Reply-To: <20070802214103.GT29424@schiele.dyndns.org>
Robert Schiele <rschiele@gmail.com> writes:
> No, you should read the mails you are refering to. I said that the
> most important stuff does work. Apparently this did not yet hurt me
> on the platform.
A non-working rebase would seem rather tough.
> Thus we have to decide whether we want some textbook example code
> and thus break this platform completely or whether we want to fix
> the issues you have listed and thus have a more portable
> application.
The "issues" are with Solaris, apparently. There is always a price
for portability. If Solaris users can fix their problems with a
global search and replace of the first line in *.sh, the question is
whether it is worth the hassle of having unreadable but "portable"
code. After all, it has to be read also by humans.
>> Sure. What about the git-rebase line using $(($end - $msgnum)) ?
>
> Bad on Solaris:
>
> $ uname -a
> SunOS solaris10-x64 5.10 Generic i86pc i386 i86pc
> $ end=1
> $ msgnum=5
> $ echo $(($end - $msgnum))
> syntax error: `(' unexpected
> $
You are missing the line
$ echo $0
which is probably the most interesting one... we don't need to be
compatible with everything having a "$ " prompt, just with everything
called "/bin/sh".
>> Too bad: this should mean that $EDITOR can get called from C... I've
>> been glad to see that so far this could be avoided.
>
> Why is it bad to call the editor from C?
See the rationale in my recently posted patch for implementing
EDITOR/VISUAL support. One needs to shell-quote stuff properly, and
the shell is better at shell-quote magic than C is.
>> I do not think we have trouble with ${parameter#word}. Much
>
> $ uname -a
> SunOS solaris10-x64 5.10 Generic i86pc i386 i86pc
> $ parameter=bla
> $ echo ${parameter#word}
> bad substitution
> $
>
>> less with ${parameter+word}; it has been in /bin/sh forever.
>
> That one is ok for Solaris.
If you prepare a patch replacing all existing ${parameter#word} uses
and get it accepted, I will not push for inclusion of my cleanup.
But you _really_ should go for it _now_.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] Handle the errors from chdir in set_work_tree
From: Alex Riesen @ 2007-08-02 22:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7v8x8ttx7y.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano, Thu, Aug 02, 2007 23:58:41 +0200:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
> > + if (chdir(dir))
> > + rel = NULL;
...
>
> Shouldn't it die() instead, though?
Dunno. Don't like dying.
> Consolidating two of your patches, would this be Ok?
Yes, but you may consider replacing strncpy with strlcpy:
> + memcpy(dir_buffer, dir, len - suffix_len);
> + dir_buffer[len - suffix_len] = '\0';
strlcpy(dir_buffer, dir, len - suffix_len + 1);
^ permalink raw reply
* Re: [PATCH] Handle the errors from chdir in set_work_tree
From: Junio C Hamano @ 2007-08-02 22:18 UTC (permalink / raw)
To: Alex Riesen; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <20070802221518.GC2829@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
> Junio C Hamano, Thu, Aug 02, 2007 23:58:41 +0200:
>> "Alex Riesen" <raa.lkml@gmail.com> writes:
>> > + if (chdir(dir))
>> > + rel = NULL;
> ...
>>
>> Shouldn't it die() instead, though?
>
> Dunno. Don't like dying.
I do not understand your reasoning. Why is it better to use
mysteriously truncated path, which may result in doing something
the user did not ask you to, rather than saying "No, my
temporary buffer is not equipped to handle such an insanely long
pathname"?
>> Consolidating two of your patches, would this be Ok?
>
> Yes, but you may consider replacing strncpy with strlcpy:
>
>> + memcpy(dir_buffer, dir, len - suffix_len);
>> + dir_buffer[len - suffix_len] = '\0';
>
> strlcpy(dir_buffer, dir, len - suffix_len + 1);
Does that buy us that much? Before going to that codepath, we
have made sure the result fits, haven't we?
^ permalink raw reply
* [PATCH] GPG Signing of Commits
From: David Soria @ 2007-08-02 22:15 UTC (permalink / raw)
To: git
Hi list,
i personally like the feature of signing commit messages using GPG. What
do you think about the patch? It's just the mechanism taken from git-tag.
Would be good not just have validation for tags, but for commits.
you found the patch at
http://experimentalworks.net/~dsp/0002-Allow-the-usage-of-GPG-signing-in-commits.patch
Hopefully you like it,
David
^ permalink raw reply
* Re: [PATCH] user-manual: mention git gui citool (commit, amend)
From: Steffen Prohaska @ 2007-08-02 22:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <20070802181853.GB31885@fieldses.org>
On Aug 2, 2007, at 8:18 PM, J. Bruce Fields wrote:
> On Mon, Jul 30, 2007 at 06:11:20PM +0200, Steffen Prohaska wrote:
>> +Another approach for creating commits is git gui:
>> +
>> +-------------------------------------------------
>> +$ git gui citool
>> +-------------------------------------------------
>> +
>> +starts the commit tool (Note, "`git gui`" starts the full gui, which
>> +provides more options).
>> +
>> +Beyond the basic operation of staging and unstaging complete files,
>> +git gui can also selectively stage diff hunks. You can do so by
>> +selecting a modified or staged file and right-click on the diff view
>> +in the lower part of the gui. A pop-up will appear that lets you
>> +select a specific hunk and stage or unstage it for the next commit.
>> +This is particular useful for slicing large, ugly commits into
>> smaller
>> +pieces, for example when cherry-picking (see
>> +<<reordering-patch-series>>).
>
> I wonder whether we could get away with just the brief list of
> features
> ("lets you view changes in the index and the working file, lets you
> individually select diff hunks for inclusion in the index"), and leave
> the how-to stuff to online guit-gui help, if it's necessary?
Maybe, this would be sufficient. I mentioned the pop-up explicitly
because it wasn't obvious to me right away. The reason might be that
my brain adjusted too much to the Mac I'm using. Right-clicking and
pop-ups are rarely used on Macs as the only access point to essential
features, such as selectively staging diff hunks. Typically an icon
or menu entry guides you explicitly to every feature. Pop-ups are only
used for 'optimized' access but never as the only access point.
> Also, I like the verb "stage" as a way to explain the part of the
> index
> file in creating commits, but I've been consistently using the word
> "index" throughout the user manual, and I think that's consistent with
> the rest of the documentation--so don't avoid it here.
"staging/unstaging" is how git gui calls adding/removing files to and
from the index.
Steffen
^ permalink raw reply
* Re: [PATCH] user-manual: mention git gui citool (commit, amend)
From: J. Bruce Fields @ 2007-08-02 22:31 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: git
In-Reply-To: <107BD473-E055-47D0-9720-9D878BDAB954@zib.de>
On Fri, Aug 03, 2007 at 12:24:10AM +0200, Steffen Prohaska wrote:
>
> On Aug 2, 2007, at 8:18 PM, J. Bruce Fields wrote:
>
>> I wonder whether we could get away with just the brief list of features
>> ("lets you view changes in the index and the working file, lets you
>> individually select diff hunks for inclusion in the index"), and leave
>> the how-to stuff to online guit-gui help, if it's necessary?
>
> Maybe, this would be sufficient. I mentioned the pop-up explicitly
> because it wasn't obvious to me right away.
I took a quick look and it wasn't obvious to me either. It'd be nice if
this could be fixed in the interface or the documentation in git-gui
itself instead of here, but perhaps saying it here is easiest for now.
>> Also, I like the verb "stage" as a way to explain the part of the index
>> file in creating commits, but I've been consistently using the word
>> "index" throughout the user manual, and I think that's consistent with
>> the rest of the documentation--so don't avoid it here.
>
> "staging/unstaging" is how git gui calls adding/removing files to and
> from the index.
I understand what you meant, but a reader of the user manual at this
point might not, since it's been consistently saying things like "to add
the contents of a new file to the index, use git add". So we should use
the same language here. Unless we want to update the whole thing to use
"stage" and "unstage". I'd rather not.
--b.
^ permalink raw reply
* Re: [PATCH] Fix documentation for core.gitproxy to reflect code
From: David Symonds @ 2007-08-02 22:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzrhvgpc.fsf@assigned-by-dhcp.cox.net>
The current implementation of core.gitproxy only operates on git:// URLs, so
the ssh:// examples and custom protocol examples have been removed or edited.
---
Documentation/config.txt | 2 +-
Documentation/git-config.txt | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3135cb7..de9e72b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -101,7 +101,7 @@ Example
# Proxy settings
[core]
- gitProxy="ssh" for "ssh://kernel.org/"
+ gitProxy="ssh" for "kernel.org"
gitProxy=default-proxy ; for the rest
Variables
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 8451ccc..c3dffff 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -214,9 +214,7 @@ Given a .git/config like this:
; Proxy settings
[core]
- gitproxy="ssh" for "ssh://kernel.org/"
gitproxy="proxy-command" for kernel.org
- gitproxy="myprotocol-command" for "my://"
gitproxy=default-proxy ; for all the rest
you can set the filemode to true with
@@ -291,7 +289,7 @@ To actually match only values with an exclamation mark, you
have to
To add a new proxy, without altering any of the existing ones, use
------------
-% git config core.gitproxy '"proxy" for example.com'
+% git config core.gitproxy '"proxy-command" for example.com'
------------
--
1.5.2.4
^ permalink raw reply related
* Re: cvs2svn conversion directly to git ready for experimentation
From: Simon 'corecode' Schubert @ 2007-08-02 22:50 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Michael Haggerty, Git Mailing List
In-Reply-To: <6715F560-FE69-4F15-8C5F-B5B6071D97ED@zib.de>
Steffen Prohaska wrote:
>> yah, that pretty much tells me it is shawn's bug :) but without more
>> details, it is very hard to diagnose.
>
> I tried again. Interestingly now togit works but tohg still fails.
>
> togit starts with reporting
>
> fatal: Not a valid object name
that's fine.
> as the first line. But besides that it seems to work fine. What
> concerns me a bit is that the last line togit reports is
>
> committing set 18100/18173
>
> I'd expect it should report 18173/18173.
that's fine as well. You only saw multiples of 100, but you didn't consider it would skip the itermediate ones, right? :)
> BTW, togit creates much more complex branching patterns than cvs2svn
> does. The attached file branching.png displays a small view of a
> branching pattern that extends downwards over a couple of screens.
> I checked the cvs2svn history again. It doesn't contain anything
> of similar complexity.
haha yea, there is still some issue with duplicate branch names and the branchpoint. if it doesn't get the branch right, it will always "pull" files from the parent branch.
did you do some manual RCS file copying or manual branch name changing of individual files? this could be the reason. I still have to find a simple repo to reproduce this.
> tohg fails (on the same repo that togit imported) with the
> following error
[..]
> AssertionError: failed to remove X/Y.cpp from manifest
This is a mercurial 0.9.3 error, as far as I can tell from the reports. This never occured here, and nobody reporting to me could ever reproduce this problem to pinpoint it.
cheers
simon
--
Serve - BSD +++ RENT this banner advert +++ ASCII Ribbon /"\
Work - Mac +++ space for low €€€ NOW!1 +++ Campaign \ /
Party Enjoy Relax | http://dragonflybsd.org Against HTML \
Dude 2c 2 the max ! http://golden-apple.biz Mail + News / \
^ permalink raw reply
* Re: Why does git-svn dcommit rewrite the log messages?
From: Sam Vilain @ 2007-08-02 19:44 UTC (permalink / raw)
To: Patrick Doyle; +Cc: git
In-Reply-To: <e2a1d0aa0708011419m1f6cb323ge9e93680147a298@mail.gmail.com>
Patrick Doyle wrote:
> I see that it appends text to the log messages with the SVN repository
> revision and ID. Does it do that because:
>
> 1) That's the _only_ way to keep git & svn in sync
> If so, then I guess I'm out of luck. But it seems (to my
> inexperienced eye) to make branching impossible in git -- as soon as I
> do a "git-svn dcommit", I lose the commit on which my branch was
> originally based. Or, am I missing something?
>
> 2) That was the simplest way to keep them in sync at the time git-svn
> was written and it hasn't bothered anybody enough to go back and fix
> it.
> If so, I'd be glad to see what I could do to fix it. I would also
> appreciate any pointers folks might care to give to get me started.
>
Primarily it's so that the person who pushed via dcommit and people who
sync via git-svn fetch get the same commit ID (assuming they are both
using the same authors map).
It might be possible to store the extra information needed to recreate
exactly the original git commit in SVN revision properties, but this
would need to be implemented once and basically not changed after that.
You can disable this behaviour, but you'll end up with duplicate commits.
Sam.
^ permalink raw reply
* Re: Shell script cleanups/style changes?
From: Junio C Hamano @ 2007-08-02 23:05 UTC (permalink / raw)
To: David Kastrup; +Cc: Robert Schiele, git
In-Reply-To: <85vebxy47e.fsf@lola.goethe.zz>
David Kastrup <dak@gnu.org> writes:
> Robert Schiele <rschiele@gmail.com> writes:
> ...
>> Thus we have to decide whether we want some textbook example code
>> and thus break this platform completely or whether we want to fix
>> the issues you have listed and thus have a more portable
>> application.
>
> The "issues" are with Solaris, apparently. There is always a price
> for portability. If Solaris users can fix their problems with a
> global search and replace of the first line in *.sh, the question is
> whether it is worth the hassle of having unreadable but "portable"
> code. After all, it has to be read also by humans.
I am in the camp of avoiding "it is even in POSIX so it's your
fault if your shell does not support it". We do not take POSIX
too seriously in that way, although we do say "let's not use it,
it is not even in POSIX". In other words, I've been trying to
be, and as a result of that we are, fairly conservative.
However, there is a line we need to draw when bending bacwards
for compatibility, and I think a system that does not have a
working command substitution $( ... ) is on the other side of
that line.
^ 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