New Git for Windows installer errors
Pat Thoyts
see two issues when upgrading from 1.7.4 to 1.7.6.
To make this installer I have started with the current devel branches
for msysgit and updated git-cheetah to its master and the docs
submodule to it's origin/maint (to ensure we have 1.7.6's docs). Then
used /share/msysgit/release.sh.
1) When the installer runs about half-way through I get an error
dialog with the following message:
c:\Program Files\Git\doc\git\html\RelNotes
An error occurred while trying to rename a file in the destination
directory: MoveFile failed; code 183. Cannot create a file when that
file already exists.
To work around this we can just choose Ignore and the installation finished ok.
Any ideas where to address this?
2) I fired up a virtual machine with windowsxp and installed git-1.7.4
then updated to 1.7.6. I see the error above and then when running Git
Bash from the desktop icon the first line printed is "ERROR: Your
msys-1.0.dll is out-of-date!"
Are we missing some update in msysgit's devel branch for msys.dll ?
Again - I'm not sure where to look to address this.
Otherwise, the test suite ran ok with the exceptions listed below.
Those prefixed with x have been present in every version. The new ones
need looking into.
git 1.7.6 2011-07-08
These tests failed:
! t1020-subdirectory (9 !alias expansion) - difference in output
between pwd in git-dir and in a subdir
! t5516-fetch-push (60 receive.denyCurrentBranch = updateInstead)
! t7406-submodule-update (3 submodule update does not fetch already
present commits)
! t7407-submodule-foreach (2 and 6)
x t9300-fast-import
x t9500-gitweb-standalone-no-errors
x t9501-gitweb-standalone-http-status
x t9502-gitweb-standalone-parse-output
x t9001-send-email
Unfinished tests:
x t9200-git-cvsexportcommit
Johannes Schindelin
On Fri, 8 Jul 2011, Pat Thoyts wrote:
> 2) I fired up a virtual machine with windowsxp and installed git-1.7.4
> then updated to 1.7.6. I see the error above and then when running Git
> Bash from the desktop icon the first line printed is "ERROR: Your
> msys-1.0.dll is out-of-date!"
You have an /etc/msys-1.0.dll.md5 which contains the wrong checksum.
/share/WinGit/release.sh (through copy-files.sh) is supposed to generate
that checksum from msysGit's own msys-1.0.dll.
> ! t5516-fetch-push (60 receive.denyCurrentBranch = updateInstead)
Argh! I forgot about that. Will try to fix ASAP...
Dscho
Pat Thoyts
> Hi,
>
> On Fri, 8 Jul 2011, Pat Thoyts wrote:
>
>> 2) I fired up a virtual machine with windowsxp and installed git-1.7.4
>> then updated to 1.7.6. I see the error above and then when running Git
>> Bash from the desktop icon the first line printed is "ERROR: Your
>> msys-1.0.dll is out-of-date!"
>
> You have an /etc/msys-1.0.dll.md5 which contains the wrong checksum.
> /share/WinGit/release.sh (through copy-files.sh) is supposed to generate
> that checksum from msysGit's own msys-1.0.dll.
OK thanks - this is caused by the md5 file having a different path
listed due to commit 42a6098.
On the XP install:
$ md5sum /bin/msys-1.0.dll | cat --show-all
2468883ebcbb731a761a0868d3b08a85 */bin/msys-1.0.dll$
$ cat --show-all msys-1.0.dll.md5
2468883ebcbb731a761a0868d3b08a85 */./bin/msys-1.0.dll$
So the MD5 is ok - the copy-files.sh script now defines MSYSGITROOT to
be /. hence the leading chars in the .md5 file. Presumably this is for
a reason. If we remove the path element will this break anything?
ie:
(md5sum $MSYSGITROOT/bin/msys-1.0.dll | cut -f1 -d' ') > etc/msys-1.0.dll.md5
and in /etc/profile use the same field extraction when comparing.
If anything else expects this .md5 file to be standard then maybe some
path substitution when generating the md5 file. When we test this it
will always be an absolute path so we might use:
$(md5sum $MSYSGITROOT/bin/msys-1.0.dll | awk '{print $1,
"*/bin/msys-1.0.dll"}') > etc/msys-1.0.dll.md5
Johannes Schindelin
Or maybe simply $(cat $MSYSGITROOT/bin/msys-1.0.dll | md5sum)?
Ciao,
Dscho
Pat Thoyts
> I've just tried making a new Git-1.7.6 installer and in testing I can
> see two issues when upgrading from 1.7.4 to 1.7.6.
> To make this installer I have started with the current devel branches
> for msysgit and updated git-cheetah to its master and the docs
> submodule to it's origin/maint (to ensure we have 1.7.6's docs). Then
> used /share/msysgit/release.sh.
>
> 1) When the installer runs about half-way through I get an error
> dialog with the following message:
> c:\Program Files\Git\doc\git\html\RelNotes
> An error occurred while trying to rename a file in the destination
> directory: MoveFile failed; code 183. Cannot create a file when that
> file already exists.
It turns out I'd broken my docs/git/html submodule earlier and had a
whole git tree in there. Fixed.
Sebastian Schuberth
> I've just tried making a new Git-1.7.6 installer and in testing I can
> see two issues when upgrading from 1.7.4 to 1.7.6.
> To make this installer I have started with the current devel branches
> for msysgit and updated git-cheetah to its master and the docs
> submodule to it's origin/maint (to ensure we have 1.7.6's docs). Then
> used /share/msysgit/release.sh.
Just to be on the same page: I guess you started with the devel branch
of both msysgit.git *and* 4msysgit.git. But why did you use "maint"
for the docs? It seems to me the 1.7.6 docs are also on "html".
> 1) When the installer runs about half-way through I get an error
> dialog with the following message:
> c:\Program Files\Git\doc\git\html\RelNotes
> An error occurred while trying to rename a file in the destination
> directory: MoveFile failed; code 183. Cannot create a file when that
> file already exists.
>
> To work around this we can just choose Ignore and the installation finished ok.
>
> Any ideas where to address this?
I just tried on a a real Vista x64. There is works for me, the
release.sh script completes without any errors.
--
Sebastian Schuberth
Sebastian Schuberth
>> Any ideas where to address this?
>
> I just tried on a a real Vista x64. There is works for me, the
> release.sh script completes without any errors.
Ah, OK, I just read your other mail.
--
Sebastian Schuberth
Sebastian Schuberth
<Johannes....@gmx.de> wrote:
>> So the MD5 is ok - the copy-files.sh script now defines MSYSGITROOT to
>> be /. hence the leading chars in the .md5 file. Presumably this is for
>> a reason. If we remove the path element will this break anything?
>> ie:
>> (md5sum $MSYSGITROOT/bin/msys-1.0.dll | cut -f1 -d' ') > etc/msys-1.0.dll.md5
>> and in /etc/profile use the same field extraction when comparing.
>> If anything else expects this .md5 file to be standard then maybe some
>> path substitution when generating the md5 file. When we test this it
>> will always be an absolute path so we might use:
>>
>> $(md5sum $MSYSGITROOT/bin/msys-1.0.dll | awk '{print $1,
>> "*/bin/msys-1.0.dll"}') > etc/msys-1.0.dll.md5
>
> Or maybe simply $(cat $MSYSGITROOT/bin/msys-1.0.dll | md5sum)?
Probably even better: In /etc/profile replace
test "$(md5sum /bin/msys-1.0.dll)" = "$(cat /etc/msys-1.0.dll.md5)"
with
md5sum -c msys-1.0.dll.md5
as this will make md5sum itself resolve the path to the file
correctly. If you agree, I'll push a commit to devel.
--
Sebastian Schuberth
Pat Thoyts
I'm getting a push rejection trying to update git master:
pt111992@UKNML4132 /git (master)
$ git push --dry-run origin master
To ssh://patt...@repo.or.cz/srv/git/msysgit.git
! [rejected] master -> master (non-fast-forward)
error: failed to push some refs to
'ssh://patt...@repo.or.cz/srv/git/msysgit.git'
I don't remember needing to use --force before. Is this to be expected
given the rebase merge straegy we use on devel. I suspect it is but I
didn't note this down so I'm checking before I break anything.
Sebastian Schuberth
> md5sum -c msys-1.0.dll.md5
>
> as this will make md5sum itself resolve the path to the file
> correctly. If you agree, I'll push a commit to devel.
More precisely, this works fine for me:
md5sum --status --check /etc/msys-1.0.dll.md5
I'll prepare a commit and push as soon as Pat agrees.
--
Sebastian Schuberth
Pat Thoyts
Thats fine. I thought about that but used the awk version for now to
avoid modifying more than just the copy-files script..
Pat.
Sebastian Schuberth
> Thats fine. I thought about that but used the awk version for now to
> avoid modifying more than just the copy-files script..
With my change, you don't need to modify "copy-files.sh" at all, but
only "profile" instead.
So are you OK with me reverting your change to "copy-files.sh" and
push my change to "profile" to devel?
--
Sebastian Schuberth
Pat Thoyts
Sure - thats fine.
Pat.
Sebastian Schuberth
>> So are you OK with me reverting your change to "copy-files.sh" and
>> push my change to "profile" to devel?
>
> Sure - thats fine.
Done.
--
Sebastian Schuberth
Johannes Schindelin
On Fri, 8 Jul 2011, Pat Thoyts wrote:
Have you checked what commits you do not yet have? (git log
master..origin/master)
Ciao,
Dscho
Johannes Schindelin
On Fri, 8 Jul 2011, Sebastian Schuberth wrote:
> On Fri, Jul 8, 2011 at 15:49, Pat Thoyts <patt...@gmail.com> wrote:
>
> >> So are you OK with me reverting your change to "copy-files.sh" and
> >> push my change to "profile" to devel?
> >
> > Sure - thats fine.
>
> Done.
I just hope that MSYSGITROOT basically always resolves to '/'... otherwise
the path for the check would be wrong :-)
Ciao,
Dscho
Pat Thoyts
Nothing is missing:
pt111992@UKNML4132 /git (master)
$ git log --oneline master..origin/master
(empty result)
It seems very odd considering the following:
pt111992@UKNML4132 /git (master)
$ git reset --hard origin/master
HEAD is now at 844cb22 start_command: flush buffers in the WIN32 code
path as well
pt111992@UKNML4132 /git (master)
$ git merge --ff-only devel
Updating 844cb22..bf1a7ff
... many lines deleted...
pt111992@UKNML4132 /git (master)
$ git push --dry-run origin master
To ssh://patt...@repo.or.cz/srv/git/msysgit.git
! [rejected] master -> master (non-fast-forward)
It successfully fast-forwarded locally but won't push?
Johannes Schindelin
Is it possible that you did not update your origin/master via "git
fetch origin"? I get this:
$ git ls-remote git://repo.or.cz/msysgit.git refs/heads/master
678125baa3dc734a72ddf067eb3c013bde556cd0 refs/heads/master
Ciao,
Dscho
Pat Thoyts
Damn - another misconfiguration on my part. It should be pushing to
/srv/git/git/mingw/4msysgit.git
Apologies for the noise. I should never have switched machines.
Johannes Schindelin
Oh! I missed that one, too! Good that we have Git refusing to
non-fast-forward by default :-)
Ciao,
Dscho
Johannes Schindelin
At least on Windows, chdir() does not update PWD. Unfortunately, stat()
does not fill any ino or dev fields anymore, so get_pwd_cwd() is not
able to tell.
But there is a telltale: both ino and dev are 0 when they are not filled
correctly, so let's be extra cautious.
This happens to fix a bug in "get-receive-pack working_directory/" when
the GIT_DIR would not be set correctly due to absolute_path(".")
returning the wrong value.
Signed-off-by: Johannes Schindelin <johannes....@gmx.de>
---
On Fri, 8 Jul 2011, Pat Thoyts wrote:
> ! t5516-fetch-push (60 receive.denyCurrentBranch = updateInstead)
This patch fixes that.
Hannes, I have no idea whether you meant 10c4c881 to fix anything
on Windows.
abspath.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/abspath.c b/abspath.c
index 01858eb..37287f8 100644
--- a/abspath.c
+++ b/abspath.c
@@ -102,7 +102,8 @@ static const char *get_pwd_cwd(void)
pwd = getenv("PWD");
if (pwd && strcmp(pwd, cwd)) {
stat(cwd, &cwd_stat);
- if (!stat(pwd, &pwd_stat) &&
+ if ((cwd_stat.st_dev || cwd_stat.st_ino) &&
+ !stat(pwd, &pwd_stat) &&
pwd_stat.st_dev == cwd_stat.st_dev &&
pwd_stat.st_ino == cwd_stat.st_ino) {
strlcpy(cwd, pwd, PATH_MAX);
--
1.7.6.rc0.4047.g15f89
Sebastian Schuberth
<Johannes....@gmx.de> wrote:
> On Fri, 8 Jul 2011, Pat Thoyts wrote:
>
> > ! t5516-fetch-push (60 receive.denyCurrentBranch = updateInstead)
>
> This patch fixes that.
I can confirm that the patch fixes test 5516 on Windows. Thanks Dscho!
--
Sebastian Schuberth
Johannes Sixt
>
> At least on Windows, chdir() does not update PWD.
Very strange wording. chdir() should not update PWD even on POSIX.
> Unfortunately, stat()
> does not fill any ino or dev fields anymore, so get_pwd_cwd() is not
> able to tell.
>
> But there is a telltale: both ino and dev are 0 when they are not filled
> correctly, so let's be extra cautious.
>
> This happens to fix a bug in "get-receive-pack working_directory/" when
> the GIT_DIR would not be set correctly due to absolute_path(".")
> returning the wrong value.
>
> Signed-off-by: Johannes Schindelin <johannes....@gmx.de>
> ---
>
> On Fri, 8 Jul 2011, Pat Thoyts wrote:
>
> > ! t5516-fetch-push (60 receive.denyCurrentBranch = updateInstead)
>
> This patch fixes that.
>
> Hannes, I have no idea whether you meant 10c4c881 to fix anything
> on Windows.
I think this fix worked for me because when git is called from CMD, PWD
is not in the enviornment and the if (pwd && ...) branch is never taken.
>
> abspath.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 01858eb..37287f8 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -102,7 +102,8 @@ static const char *get_pwd_cwd(void)
> pwd = getenv("PWD");
> if (pwd && strcmp(pwd, cwd)) {
> stat(cwd, &cwd_stat);
> - if (!stat(pwd, &pwd_stat) &&
> + if ((cwd_stat.st_dev || cwd_stat.st_ino) &&
> + !stat(pwd, &pwd_stat) &&
> pwd_stat.st_dev == cwd_stat.st_dev &&
> pwd_stat.st_ino == cwd_stat.st_ino) {
> strlcpy(cwd, pwd, PATH_MAX);
Acked-by: Johannes Sixt <j...@kdbg.org>
-- Hannes
Johannes Schindelin
On Sun, 10 Jul 2011, Johannes Sixt wrote:
> Am 09.07.2011 19:38, schrieb Johannes Schindelin:
> >
> > At least on Windows, chdir() does not update PWD.
>
> Very strange wording. chdir() should not update PWD even on POSIX.
Well, you might think it is strange wording. But then, it expresses
exactly what I meant it to say. chdir() does not update PWD on Windows.
You might be very surprised, but that is not true on the Linux system
where one of the 4msysgit.git test cases does _not_ break, while it does
on Windows.
I hoped to make that clear with the wording, but apparently I failed
rather blatantly.
All the more surprising do I find that:
> Acked-by: Johannes Sixt <j...@kdbg.org>
Ciao,
Johannes
Randal L. Schwartz
Johannes> You might be very surprised, but that is not true on the Linux system
Johannes> where one of the 4msysgit.git test cases does _not_ break, while it does
Johannes> on Windows.
If you ever depend on a userspace PWD to be your actual current
directory without at least stat()ing it, you've failed.
In my experience, it is *never* reliable. It's just a hint.
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<mer...@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.posterous.com/ for Smalltalk discussion
Johannes Schindelin
On Sun, 10 Jul 2011, Randal L. Schwartz wrote:
> >>>>> "Johannes" == Johannes Schindelin <Johannes....@gmx.de> writes:
>
> Johannes> You might be very surprised, but that is not true on the Linux
> Johannes> system where one of the 4msysgit.git test cases does _not_
> Johannes> break, while it does on Windows.
>
> If you ever depend on a userspace PWD to be your actual current
> directory without at least stat()ing it, you've failed.
>
> In my experience, it is *never* reliable. It's just a hint.
To be precise, get_pwd_cwd() _does_ stat() what's in PWD, and _does_
compare with the stat() of what comes out of getcwd(), but that comparison
uses only st_dev and st_ino, both of which happen to be 0 in my case --
for each and every file/directory.
I can only _guess_ at the reasoning behind get_pwd_cwd(). I _think_ it was
meant to catch the case when getcwd() and PWD refer to the same directory,
but PWD goes through symbolic links. I was tempted to just throw that PWD
handling out for Windows, since we do not have symbolic link handling yet.
But that is currently actively discussed, so we might need it in the
future, in which case I have to figure out how to fake reliable st_dev and
st_ino values into our stat() code.
Ciao,
Dscho
Junio C Hamano
> On Sun, 10 Jul 2011, Randal L. Schwartz wrote:
> ...
>> If you ever depend on a userspace PWD to be your actual current
>> directory without at least stat()ing it, you've failed.
>>
>> In my experience, it is *never* reliable. It's just a hint.
>
> To be precise, get_pwd_cwd() _does_ stat() what's in PWD, and _does_
> compare with the stat() of what comes out of getcwd(), but that comparison
> uses only st_dev and st_ino, both of which happen to be 0 in my case --
> for each and every file/directory.
>
> I can only _guess_ at the reasoning behind get_pwd_cwd(). I _think_ it was
> meant to catch the case when getcwd() and PWD refer to the same directory,
> but PWD goes through symbolic links.
Thanks for a much clearer explanation than before. I tried to reword the
proposed commit log message using the description above.
I feel that the title is still not optimal. If the original code used to
return getenv("PWD") if the environment variable is set, and otherwise
fell back to getcwd(), and the updated code tries to make sure they refer
to the same directory, then "Do not trust PWD blindly" would be a good
description for the fix, but the code you fixed the bug in tried not to
trust PWD blindly but failed to realize that on some systems dev/ino field
may be unreliable.
"Do not trust st.st_ino/st.st_dev blindly" might be a better title in that
sense.
In any case, thanks for a fix; will queue.
Author: Johannes Schindelin <Johannes....@gmx.de>
Date: Sat Jul 9 19:38:08 2011 +0200
Do not trust PWD blindly
10c4c88 (Allow add_path() to add non-existent directories to the path,
2008-07-21) introduced get_pwd_cwd() function in order to favor $PWD when
getenv("PWD") and getcwd() refer to the same directory but are different
strings (e.g. the former gives a nicer looking name via a symbolic link to
an uglier looking automounted path). The function tried to determine if
two directories are the same by running stat(2) on both and comparing
ino/dev fields.
Unfortunately, stat() does not fill any ino or dev fields in msysgit. But
there is a telltale: both ino and dev are 0 when they are not filled
correctly, so let's be extra cautious.
This happens to fix a bug in "get-receive-pack working_directory/" when
the GIT_DIR would not be set correctly due to absolute_path(".")
returning the wrong value.
Signed-off-by: Johannes Schindelin <johannes....@gmx.de>
Acked-by: Johannes Sixt <j...@kdbg.org>
Signed-off-by: Junio C Hamano <git...@pobox.com>
Johannes Schindelin
On Mon, 11 Jul 2011, Junio C Hamano wrote:
> "Do not trust st.st_ino/st.st_dev blindly" might be a better title in
> that sense.
Maybe prefix it with "get_pwd_cwd(): "?
> In any case, thanks for a fix; will queue.
Thanks!
Johannes