* Re: branch ahead in commits but push claims all up to date
From: John Tapsell @ 2009-03-25 2:13 UTC (permalink / raw)
To: Irene Ros; +Cc: Daniel Barkalow, git
In-Reply-To: <7001b7a00903241901w107e2973i9912eab114c9cde0@mail.gmail.com>
2009/3/25 Irene Ros <imirene@gmail.com>:
> Hi All,
>
> Thank you for the good advice. I may be the case I am somehow misusing
So, did you actually try:
git fetch
?
> git... I couldn't resolve the issue and so I created a new project off
> of the same repo. Switching to the same branch in question yielded an
> even stranger result: In this new project, the commits were there (I
> could see them in git log and in git log origin/myBranch) whereas in
> the previous older project I did not... does that make sense? Our
> origin branches are located on a central server so can't quite figure
> out why viewing the log of the same remote branch from two different
> projects would yield different results. Any suggestions? At this
> point, I'm just really curious.
>
> -- Irene
>
>
>
> On Tue, Mar 24, 2009 at 9:24 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
>> On Wed, 25 Mar 2009, John Tapsell wrote:
>>
>>> 2009/3/24 Daniel Barkalow <barkalow@iabervon.org>:
>>> > On Tue, 24 Mar 2009, John Tapsell wrote:
>>> >
>>> >> 2009/3/24 Irene Ros <imirene@gmail.com>:
>>> >> > Hi all,
>>> >> >
>>> >> > I've been using git for some time now and haven't run into this issue
>>> >> > before, perhaps someone else here has:
>>> >> >
>>> >> > I have a branch that is ahead of its origin by a few commits:
>>> >> >
>>> >> > $ git status
>>> >> > # On branch myBranch
>>> >> > # Your branch is ahead of 'origin/myBranch' by 10 commits.
>>> >>
>>> >> Tried running: git fetch ?
>>> >>
>>> >> For some weird reason "git push origin mybranch" doesn't actually
>>> >> update origin/mybranch. It's more annoying :-)
>>> >
>>> > It should, so long as you're using the native transport and
>>> > origin/mybranch actually tracks mybranch on origin.
>>> >
>>> > "git push" doesn't update it, but the code that implements the native
>>> > transport does update it if it succeeds.
>>> >
>>> > (Actually, I'm not 100% sure that, if you update origin through some other
>>> > channel with exactly the commit that you now have in mybranch locally, and
>>> > then try the push, it will update the local tracking for that branch; is
>>> > that what you've hit?)
>>>
>>> I update via http - maybe that's why? origin/mybranch is never
>>> updated when I push. It's not just a once-off quirk.
>>
>> Yup, http doesn't have it. One of my series currently in next moves it
>> from the git-specific protocol to the common code, but there's still work
>> to be done to allow the http push transport to report back to the common
>> code what got updated successfully, which is largely a matter of making
>> the http-push code run in-process instead of as a command run by
>> transport.c, and using the just-added API.
>>
>> -Daniel
>> *This .sig left intentionally blank*
>
^ permalink raw reply
* Re: [BUG?] How to make a shared/restricted repo?
From: Johan Herland @ 2009-03-25 2:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvdpybf9i.fsf@gitster.siamese.dyndns.org>
On Wednesday 25 March 2009, Junio C Hamano wrote:
> You might like to try a patch like this (untested).
>
> path.c | 17 +++++------------
> 1 files changed, 5 insertions(+), 12 deletions(-)
Thanks!
This works much better :)
However, there are still some questions/issues:
- t1301-shared-repo.sh fails:
Oops, .git/HEAD is not 0664 but -rw-rw---- [...]
* FAIL 3: shared=1 does not clear bits preset by umask 022
(I guess this is expected, as your patch changes the assumptions)
- Loose objects and pack files are still world-readable.
- Shared repos are no longer world-readable by default (requires
"--shared=all" to be world-readable). This might be
confusing to old users with lenient umasks, and should probably be
mentioned in the documentation. AFAICS, it also has nasty effects
on existing repos with core.sharedRepository == 1. We should probably
re-roll the patch adding a new keyword (e.g. "group-only"), instead of
changing the semantics of existing keywords/modes.
- Files/dirs copied from template directory are still world-readable.
(This is not a big deal at all)
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* [PATCH] init-db: support --import to add all files and commit right after init
From: Nguyễn Thái Ngọc Duy @ 2009-03-25 2:09 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is equivalent to "git init;git add .;git commit -q -m blah".
I find myself doing that too many times, hence this shortcut.
In future, --fast-import support would also be nice if the import
directory has a lot of files.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I'm not really sure about document format as I failed to install
xmlto on my machine. Html output looked fine though.
Documentation/git-init.txt | 16 +++++++++++++-
builtin-init-db.c | 49 ++++++++++++++++++++++++++++++++++++++++---
t/t0001-init.sh | 32 ++++++++++++++++++++++++++++
3 files changed, 92 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 71749c0..52af645 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -8,7 +8,8 @@ git-init - Create an empty git repository or reinitialize an existing one
SYNOPSIS
--------
-'git init' [-q | --quiet] [--bare] [--template=<template_directory>] [--shared[=<permissions>]]
+'git init' [-q | --quiet] [--bare] [--template=<template_directory>]
+ [--shared[=<permissions>]] [-m <message> | --import=<message>]
OPTIONS
@@ -68,6 +69,19 @@ By default, the configuration flag receive.denyNonFastForwards is enabled
in shared repositories, so that you cannot force a non fast-forwarding push
into it.
+-m <message>::
+--import=<message>::
+
+Commit everything to the newly initialized repository. This is equivalent to:
+
+----------------
+$ git init
+$ git add .
+$ git commit -q -m <message>
+----------------
+
+This option does not work with `--bare` nor already initialized repository.
+
--
diff --git a/builtin-init-db.c b/builtin-init-db.c
index ee3911f..5bf563a 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -6,6 +6,7 @@
#include "cache.h"
#include "builtin.h"
#include "exec_cmd.h"
+#include "run-command.h"
#ifndef DEFAULT_GIT_TEMPLATE_DIR
#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -19,6 +20,7 @@
static int init_is_bare_repository = 0;
static int init_shared_repository = -1;
+static int reinit;
static void safe_create_dir(const char *dir, int share)
{
@@ -279,7 +281,7 @@ int init_db(const char *template_dir, unsigned int flags)
{
const char *sha1_dir;
char *path;
- int len, reinit;
+ int len;
safe_create_dir(get_git_dir(), 0);
@@ -363,8 +365,29 @@ static int guess_repository_type(const char *git_dir)
return 1;
}
+static int import_files(const char *import_message)
+{
+ const char *args[6];
+ int i = 0;
+ int ret;
+
+ args[i++] = "add";
+ args[i++] = ".";
+ args[i] = NULL;
+ ret = run_command_v_opt(args, RUN_GIT_CMD);
+ if (ret)
+ return ret;
+ i = 0;
+ args[i++] = "commit";
+ args[i++] = "-q";
+ args[i++] = "-m";
+ args[i++] = import_message;
+ args[i] = NULL;
+ return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
static const char init_db_usage[] =
-"git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]]";
+"git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]] [-m <message> | --import=<message>]";
/*
* If you want to, you can share the DB area with any number of branches.
@@ -377,7 +400,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
const char *git_dir;
const char *template_dir = NULL;
unsigned int flags = 0;
- int i;
+ const char *import_message = NULL;
+ int ret, i;
for (i = 1; i < argc; i++, argv++) {
const char *arg = argv[1];
@@ -392,12 +416,24 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
init_shared_repository = PERM_GROUP;
else if (!prefixcmp(arg, "--shared="))
init_shared_repository = git_config_perm("arg", arg+9);
+ else if (!strcmp(arg, "--import") || !strcmp(arg, "-m")) {
+ if (i+1 >= argc)
+ die("--import requires an import message");
+ import_message = argv[2];
+ i++;
+ argv++;
+ }
+ else if (!prefixcmp(arg, "--import="))
+ import_message = arg+9;
else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
flags |= INIT_DB_QUIET;
else
usage(init_db_usage);
}
+ if (import_message && is_bare_repository_cfg == 1)
+ die("--import does not work with --bare");
+
/*
* GIT_WORK_TREE makes sense only in conjunction with GIT_DIR
* without --bare. Catch the error early.
@@ -440,5 +476,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
set_git_dir(make_absolute_path(git_dir));
- return init_db(template_dir, flags);
+ ret = init_db(template_dir, flags);
+ if (ret || !import_message)
+ return ret;
+ if (reinit)
+ die("--import does not work with already initialized repository");
+ return import_files(import_message);
}
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 5ac0a27..86414f0 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -199,4 +199,36 @@ test_expect_success 'init honors global core.sharedRepository' '
x`git config -f shared-honor-global/.git/config core.sharedRepository`
'
+test_expect_success 'init --import with --bare' '
+ (
+ unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+ mkdir init-import-bare.git &&
+ cd init-import-bare.git &&
+ ! git init --bare --import test
+ )
+
+'
+
+test_expect_success 'init --import' '
+ (
+ test_tick
+ unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+ mkdir init-import &&
+ cd init-import &&
+ touch foo bar &&
+ git init --import test &&
+ test "$(git rev-parse HEAD)" = 758aa5a579e42200a6fd4e4964c7e1dc1875d67d
+ )
+'
+
+test_expect_success 'reinit --import' '
+ (
+ test_tick
+ unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+ cd init-import &&
+ ! git init --import testagain &&
+ test "$(git rev-parse HEAD)" = 758aa5a579e42200a6fd4e4964c7e1dc1875d67d
+ )
+'
+
test_done
--
1.6.1.446.gc7851
^ permalink raw reply related
* Re: branch ahead in commits but push claims all up to date
From: Irene Ros @ 2009-03-25 2:01 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: John Tapsell, git
In-Reply-To: <alpine.LNX.1.00.0903242118270.19665@iabervon.org>
Hi All,
Thank you for the good advice. I may be the case I am somehow misusing
git... I couldn't resolve the issue and so I created a new project off
of the same repo. Switching to the same branch in question yielded an
even stranger result: In this new project, the commits were there (I
could see them in git log and in git log origin/myBranch) whereas in
the previous older project I did not... does that make sense? Our
origin branches are located on a central server so can't quite figure
out why viewing the log of the same remote branch from two different
projects would yield different results. Any suggestions? At this
point, I'm just really curious.
-- Irene
On Tue, Mar 24, 2009 at 9:24 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Wed, 25 Mar 2009, John Tapsell wrote:
>
>> 2009/3/24 Daniel Barkalow <barkalow@iabervon.org>:
>> > On Tue, 24 Mar 2009, John Tapsell wrote:
>> >
>> >> 2009/3/24 Irene Ros <imirene@gmail.com>:
>> >> > Hi all,
>> >> >
>> >> > I've been using git for some time now and haven't run into this issue
>> >> > before, perhaps someone else here has:
>> >> >
>> >> > I have a branch that is ahead of its origin by a few commits:
>> >> >
>> >> > $ git status
>> >> > # On branch myBranch
>> >> > # Your branch is ahead of 'origin/myBranch' by 10 commits.
>> >>
>> >> Tried running: git fetch ?
>> >>
>> >> For some weird reason "git push origin mybranch" doesn't actually
>> >> update origin/mybranch. It's more annoying :-)
>> >
>> > It should, so long as you're using the native transport and
>> > origin/mybranch actually tracks mybranch on origin.
>> >
>> > "git push" doesn't update it, but the code that implements the native
>> > transport does update it if it succeeds.
>> >
>> > (Actually, I'm not 100% sure that, if you update origin through some other
>> > channel with exactly the commit that you now have in mybranch locally, and
>> > then try the push, it will update the local tracking for that branch; is
>> > that what you've hit?)
>>
>> I update via http - maybe that's why? origin/mybranch is never
>> updated when I push. It's not just a once-off quirk.
>
> Yup, http doesn't have it. One of my series currently in next moves it
> from the git-specific protocol to the common code, but there's still work
> to be done to allow the http push transport to report back to the common
> code what got updated successfully, which is largely a matter of making
> the http-push code run in-process instead of as a command run by
> transport.c, and using the just-added API.
>
> -Daniel
> *This .sig left intentionally blank*
^ permalink raw reply
* Re: large(25G) repository in git
From: Nicolas Pitre @ 2009-03-25 1:47 UTC (permalink / raw)
To: Adam Heath; +Cc: Sam Hocevar, git
In-Reply-To: <49C9818A.9040507@brainfood.com>
On Tue, 24 Mar 2009, Adam Heath wrote:
> Nicolas Pitre wrote:
> > On Tue, 24 Mar 2009, Adam Heath wrote:
> >
> >> Sam Hocevar wrote:
> >>> In your particular case, I would suggest setting pack.packSizeLimit
> >>> to something lower. This would reduce the time spent generating a new
> >>> pack file if the problem were to happen again.
> >> Yeah, saw that one, but *after* I had this problem. The default, if
> >> not set, is unlimited, which in this case, is definately *not* what we
> >> want.
> >
> > In your particular case, if the problem is actually what I think it is,
> > the pack.packSizeLimit wouldn't have made any difference. This setting
> > affects local repacking only and has no effect what so ever on the push
> > operation.
>
> Ooh. Care to enlighten those of us not blessed with git internal
> knowledge?
See my previous email for a likely explanation about your issue.
As to the pack.packSizeLimit setting: it is used when repacking only in
order to avoid big packs on systems that might have issues dealing with
large files. During a repack, if the currently produced pack is about
to get over that limit, then the pack is closed and a new one is
started. You therefore end up with many packs.
The transfer protocol used during a fetch or a push uses the pack format
streamed over the network, but only one pack can be transferred that
way. Maybe the reception of a pack during a network transfer should be
split according to pack.packSizeLimit as well, but this is currently not
implemented at all. No one complained about that either, so I'm guessing
that
splitting a large pack, if needed, by using 'git repack' after a
clone/fetch is good enough.
Personally, I don't think actively splitting packs into smaller ones is
that useful, unless you wish to archive them on a file system which
cannot handle files larger than 2GB or the like.
> On another note, anyone have a goat I can buy, for the sacrifice?
Beware the wrath of Git...
Nicolas
^ permalink raw reply
* Re: branch ahead in commits but push claims all up to date
From: Daniel Barkalow @ 2009-03-25 1:24 UTC (permalink / raw)
To: John Tapsell; +Cc: Irene Ros, git
In-Reply-To: <43d8ce650903241726s122cc468q4ea9188e1561832@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1794 bytes --]
On Wed, 25 Mar 2009, John Tapsell wrote:
> 2009/3/24 Daniel Barkalow <barkalow@iabervon.org>:
> > On Tue, 24 Mar 2009, John Tapsell wrote:
> >
> >> 2009/3/24 Irene Ros <imirene@gmail.com>:
> >> > Hi all,
> >> >
> >> > I've been using git for some time now and haven't run into this issue
> >> > before, perhaps someone else here has:
> >> >
> >> > I have a branch that is ahead of its origin by a few commits:
> >> >
> >> > $ git status
> >> > # On branch myBranch
> >> > # Your branch is ahead of 'origin/myBranch' by 10 commits.
> >>
> >> Tried running: git fetch ?
> >>
> >> For some weird reason "git push origin mybranch" doesn't actually
> >> update origin/mybranch. It's more annoying :-)
> >
> > It should, so long as you're using the native transport and
> > origin/mybranch actually tracks mybranch on origin.
> >
> > "git push" doesn't update it, but the code that implements the native
> > transport does update it if it succeeds.
> >
> > (Actually, I'm not 100% sure that, if you update origin through some other
> > channel with exactly the commit that you now have in mybranch locally, and
> > then try the push, it will update the local tracking for that branch; is
> > that what you've hit?)
>
> I update via http - maybe that's why? origin/mybranch is never
> updated when I push. It's not just a once-off quirk.
Yup, http doesn't have it. One of my series currently in next moves it
from the git-specific protocol to the common code, but there's still work
to be done to allow the http push transport to report back to the common
code what got updated successfully, which is largely a matter of making
the http-push code run in-process instead of as a command run by
transport.c, and using the just-added API.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: large(25G) repository in git
From: Nicolas Pitre @ 2009-03-25 1:21 UTC (permalink / raw)
To: Adam Heath; +Cc: git
In-Reply-To: <49C948C1.2070404@brainfood.com>
On Tue, 24 Mar 2009, Adam Heath wrote:
> Nicolas Pitre wrote:
> > As much as I would like to believe you, this doesn't help fixing the
> > problem if you don't provide more information about this. For example,
> > the output from git during the whole operation might give us the
> > beginning of a clue. Otherwise, all I can tell you is that such thing
> > is not supposed to happen.
>
> First off, you've put a bad tone on this. It appears that you are
> saying I'm mistaken, and it didn't send all that data. "It can't
> happen, so it didn't happen." Believe me, if it hadn't resent all
> this data, I wouldn't have even sent the email.
I don't know you. All I had is the information you provided which was
rather incomplete. So don't be offended if I ask for more. I'm trying
to help you after all.
And especially in this case, the problem seems not to be about
packing...
> In any event, we got lucky. I *do* have a log of the push side of
> this problem. I doubt it's enough to figure out the actual cause tho.
Well, I think it might.
> ==
> Counting objects: 96637, done.
> Compressing objects: 100% (29670/29670), done.
> Writing objects: 100% (96637/96637), 25.49 GiB | 226 KiB/s, done.
> Total 96637 (delta 48713), reused 96637 (delta 48713)
> To ssh://bf-yum/@anon-site@
> * [new branch] master -> lnxwww10
Was that branch really new on the remote side? If no, then this is
highly suspicious. If somehow the previously aborted push attempt
screwed the remote refs, then the local client would think that the
remote is empty and conclude that all commits have to be pushed.
> >> After I ran git push, ssh timed out, the temp pack that was created
> >> was then removed, as git complained about the connection being gone.
> >
> > On a push, there is no creation of a temp pack. It is always produced
> > on the fly and pushed straight via the ssh connection.
>
> No. I saw a temp file in strace. It *was* created on the local disk,
> and *not* sent on the fly.
A temp pack is created on the receiving side, not the sending side
though. The sending side is piping the pack data on its standard output
which is connected to ssh's standard input.
> >> Um, if it's missing documentation, then how am I supposed to know
> >> about it?
> >
> > Asking on the list, like you did. However this attribute should be
> > documented as well of course. I even think that someone posted a patch
> > for it a while ago which might have been dropped.
>
> What I'd like, is a way to say a certain pattern of files should only
> be deduped, and not deltafied. This would handle the case of exact
> copies, or renames, which would still be a win for us, but generally
> when a new video(or doc or pdf) is uploaded, it's alot of work to try
> and deltafy, for very little benefit.
Renamed/duplicated files are always stored uniquely by design. Git
store file data into objects which are named after the SHA1 of their
content.
In order to not attempt any delta on PDF files for example, you need to
add a negative delta attribute line such as:
*.pdf -delta
either in a file called .gitattributes which gets versionned
and distributed, or in .git/info/attributes in which case it'll remain
local. Any file matching *.pdf won't be delta compressed.
Nicolas
^ permalink raw reply
* Re: large(25G) repository in git
From: Adam Heath @ 2009-03-25 0:57 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Sam Hocevar, git
In-Reply-To: <alpine.LFD.2.00.0903242025110.26337@xanadu.home>
Nicolas Pitre wrote:
> On Tue, 24 Mar 2009, Adam Heath wrote:
>
>> Sam Hocevar wrote:
>>> In your particular case, I would suggest setting pack.packSizeLimit
>>> to something lower. This would reduce the time spent generating a new
>>> pack file if the problem were to happen again.
>> Yeah, saw that one, but *after* I had this problem. The default, if
>> not set, is unlimited, which in this case, is definately *not* what we
>> want.
>
> In your particular case, if the problem is actually what I think it is,
> the pack.packSizeLimit wouldn't have made any difference. This setting
> affects local repacking only and has no effect what so ever on the push
> operation.
Ooh. Care to enlighten those of us not blessed with git internal
knowledge?
On another note, anyone have a goat I can buy, for the sacrifice?
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Jeff Layton @ 2009-03-25 0:49 UTC (permalink / raw)
To: Steven French; +Cc: Git Mailing List, Linus Torvalds, Peter
In-Reply-To: <OFBC676D76.DF7BF27E-ON87257584.0000C3CB-86257584.0001BE68@us.ibm.com>
On Tue, 24 Mar 2009 19:17:34 -0500
Steven French <sfrench@us.ibm.com> wrote:
> Jeff Layton <jlayton@redhat.com> wrote on 03/24/2009 06:35:06 PM:
>
> > On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> > >
> > >
> > > On Tue, 24 Mar 2009, Peter wrote:
> > > > >
> > > > > What OS? What filesystem? Are you perhaps running out of space?
> > > >
> > > > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs
> > ( this is a
> > > > windows 2000 share mounted with samba in a VMware Workstation
> > Debian client (
> > > > yes, I know ... )). Memory usage, according to htop, is constant
> > = 140/504 MB
> > > > during the whole process until git fails.
> > >
> > > Ok, it sounds very much like a possible CIFS problem.
> > >
> > > Getting the exact error code for the "close()" will be interesting,
> > > because the only thing that can return an error under Linux in close()
> is
> > > if the filesystem "->flush()" function fails with an error.
> > >
> > > In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing
> the
> > > data out, and that in turn calls do_writepages() which in turn calls
> > > either generic_writepages() or cifs_writepages() depending on random
> cifs
> > > crap.
> > >
> > > I'm not seeing any obvious errors there. But I would _not_ be
> surprised if
> > > the fchmod(fd, 0444) that we did before the close could be causing
> this:
> > > cifs gets confused and thinks that it must not write to the file
> because
> > > the file has been turned read-only.
> > >
> > > Here's an idea: if this is reproducible for you, does the behavior
> change
> > > if you do
> > >
> > > [core]
> > > core.fsyncobjectfiles = true
> > >
> > > in your .git/config file? That causes git to always fsync() the
> written
> > > data _before_ it does that fchmod(), which in turn means that by the
> time
> > > the close() rolls around, there should be no data to write, and thus
> no
> > > possibility that anybody gets confused when there is still dirty data
> on a
> > > file that has been marked read-only.
> > >
> > > Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS
> go-to
> > > guys. It does seem like a CIFS bug is likely.
> > >
> > > Steve, Jeff: git does basically
> > >
> > > open(".git/objects/xy/tmp_obj_xyzzy", O_RDWR|O_CREAT|O_EXCL, 0600)
> = 5
> > > write(5, ".."..., len)
> > > fchmod(5, 0444)
> > > close(5)
> > > link(".git/objects/xy/tmp_obj_xyzzy", ".git/objects/xy/xyzzy");
> > > unlink(".git/objects/xy/tmp_obj_xyzzy");
> > >
> > > to write a new datafile. Under CIFS, that "close()" apparently
> sometimes
> > > returns with an error, and fails.
> > >
> > > I guess we could change the "fchmod()" to happen after the close(),
> just
> > > to make it easier for filesystems to get this right. And yes, as
> outlined
> > > above, there's a config option to make git use fdatasync() before it
> does
> > > that fchmod() too. But it does seem like CIFS is just buggy.
> > >
> > > If CIFS has problems with the above sequence (say, if some timeout
> > > refreshes the inode data or causes a re-connect with the server or
> > > whatever), then maybe cifs should always do an implicit fdatasync()
> when
> > > doing fchmod(), just to make sure that the fchmod won't screw up any
> > > cached dirty data?
> > >
> >
> > Yes. That's probably the right thing to do here. This is looks like a
> > regression that I introduced some time ago in
> > cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always
> > flushed all the data before doing any setattr. After that delta, we
> > just did it on size changes. In a later commit, Steve fixed it so that
> > it got done on ATTR_MTIME too.
> >
> > We can easily change the cifs_setattr variants to flush all the dirty
> > data on ATTR_MODE changes, and maybe even change it to only flush on
> > certain ATTR_MODE changes, like when we're clearing all the write bits.
> >
> > I wonder though whether that's sufficient. If we're changing ownership
> > for instance, will we hit the same issue? Maybe the safest approach is
> > just to go back to flushing on any setattr call.
> >
> > Steve, thoughts?
>
> Flushing on all setattr calls seems excessive, but the case of
> fchmod(0444) followed by close should not require it, and should
> work over cifs unless the tcp socket drops between the chmod
> and the close/flush (requiring reconnection and reopening
> the file handle). For the normal case, the chmod should not
> matter, since for cifs, unlike NFSv3, there is a network
> open operation and access checks are done on the open call, and
> you already have the file open for write). If /proc/fs/cifs/Stats
> shows the number of reconnects as 0, then we don't have the case
> of dropping the network between chmod and flush/close. Since certain
> writebehind errors (in writepages) can not be returned to the application
> until flush/close, it is just as likely that what you are seeing is
> an error from a previous failed write. The logic for handling
> errors in cifs_writepages is much better in recent kernels.
>
> What is the kernel version?
>
Good point -- I stand corrected. I just gave this series of calls a quick
test against a win2k8 server:
unlink("/mnt/win2k8/testfile") = 0
open("/mnt/win2k8/testfile", O_RDWR|O_CREAT, 0644) = 3
write(3, "\1\0\0\0\0\0\0\0\35\244@\211:\0\0\0\7\0\0\0\0\0\0\0X\r\25-\377\177\0\0`"..., 1024) = 1024
fchmod(3, 0444) = 0
close(3) = 0
Here's what it looks like on the wire (forgive the IPv6 addresses, it's
just what I had handy)...
Create the file:
3.842955 feed::2 -> feed::8 SMB NT Create AndX Request, Path: \testfile
3.843641 feed::8 -> feed::2 SMB NT Create AndX Response, FID: 0x4007
Set the ATTR_READONLY bit:
3.844255 feed::2 -> feed::8 SMB Trans2 Request, SET_FILE_INFO, FID: 0x4007
3.845070 feed::8 -> feed::2 SMB Trans2 Response, FID: 0x4007, SET_FILE_INFO
Write the data:
3.845766 feed::2 -> feed::8 SMB Write AndX Request, FID: 0x4007, 1024 bytes at offset 0
3.846627 feed::8 -> feed::2 SMB Write AndX Response, FID: 0x4007, 1024 bytes
Close the file:
3.847043 feed::2 -> feed::8 SMB Close Request, FID: 0x4007
3.847940 feed::8 -> feed::2 SMB Close Response, FID: 0x4007
...none of that errored out, even though the writes clearly hit the
server after the mode change. Either there's something else going on
here, or this server is misbehaving. It might be interesting to see a
capture.
Some info about what sort of server this is might also be helpful.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply
* Re: [BUG?] How to make a shared/restricted repo?
From: Junio C Hamano @ 2009-03-25 0:49 UTC (permalink / raw)
To: Brandon Casey; +Cc: Johan Herland, git
In-Reply-To: <sL3rt6iQWyznVMwP2SukD7BiuS1AVuqwVkMR4XSwA5SnK9TLmqyqAg@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> Johan Herland wrote:
>> Hi,
>>
>> Some colleagues of mine are working on a "secret" project, and they want to
>> create a central/server/integration repo that should be group-writable, but
>> not at all accessible to anybody outside the group (i.e. files should be
>> 0660 ("-rw-rw----"), dirs should be 0770 ("drwxrws---")).
>>
>> I started setting this up for them in the following manner:
>>
>> mkdir foo.git
>> cd foo.git
>> git init --bare --shared=group
>> cd ..
>> chgrp -R groupname foo.git
>> chmod -R o-rwx foo.git
>>
>> ...and everything looks good, initially...
>>
>> However, when I start pushing into this repo, the newly created files are
>> readable to everybody (files are 0664 ("-rw-rw-r--"), dirs are 0775
>> ("drwxrwsr-x")).
>
> But nobody has access to anything under foo.git since you did
> 'chmod o-rwx foo.git' above.
>
> Unless I'm missing something, I think you already have what you want.
The toplevel is never recreated so it should be Ok in practice.
The core.sharedrepository only loosens the effect of overtight umask
setting that a project member has. But you can notice inconsistency when
you run "ls -l", which may bother you ;-)
^ permalink raw reply
* Re: [BUG?] How to make a shared/restricted repo?
From: Junio C Hamano @ 2009-03-25 0:46 UTC (permalink / raw)
To: Johan Herland; +Cc: git
In-Reply-To: <200903250105.05808.johan@herland.net>
Johan Herland <johan@herland.net> writes:
> core.sharedRepository
> [...] When 0xxx, where 0xxx is an octal number, files in the repository
> will have this mode value. 0xxx will override user’s umask value, and
> thus, users with a safe umask (0077) can use this option. [...]
Traditionally, core.shreadrepository was used only to widen overtight
umask some paranoid users have that are antisocial in a group project
setting. An object that happens to have created by a lenient member may
be readable by others, but another object created by a member with
stricter umask won't be, which means "core.sharedRepository = group" means
just that; it guarantees that it is at least usable by everybody in the
group even if there is somebody antisocial in the group, and it does not
say anything about accessibility by others at all.
You might like to try a patch like this (untested).
path.c | 17 +++++------------
1 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/path.c b/path.c
index e332b50..fbc7011 100644
--- a/path.c
+++ b/path.c
@@ -314,7 +314,7 @@ char *enter_repo(char *path, int strict)
int adjust_shared_perm(const char *path)
{
struct stat st;
- int mode;
+ int mode, tweak;
if (!shared_repository)
return 0;
@@ -322,17 +322,10 @@ int adjust_shared_perm(const char *path)
return -1;
mode = st.st_mode;
- if (shared_repository) {
- int tweak = shared_repository;
- if (!(mode & S_IWUSR))
- tweak &= ~0222;
- mode |= tweak;
- } else {
- /* Preserve old PERM_UMASK behaviour */
- if (mode & S_IWUSR)
- mode |= S_IWGRP;
- }
-
+ tweak = shared_repository;
+ if (!(mode & S_IWUSR))
+ tweak &= ~0222;
+ mode = (mode & ~0777) | tweak;
if (S_ISDIR(mode)) {
mode |= FORCE_DIR_SET_GID;
^ permalink raw reply related
* Re: [BUG?] How to make a shared/restricted repo?
From: Johan Herland @ 2009-03-25 0:45 UTC (permalink / raw)
To: Brandon Casey; +Cc: git
In-Reply-To: <sL3rt6iQWyznVMwP2SukD7BiuS1AVuqwVkMR4XSwA5SnK9TLmqyqAg@cipher.nrlssc.navy.mil>
On Wednesday 25 March 2009, Brandon Casey wrote:
> Johan Herland wrote:
> > Some colleagues of mine are working on a "secret" project, and they
> > want to create a central/server/integration repo that should be
> > group-writable, but not at all accessible to anybody outside the group
> > (i.e. files should be 0660 ("-rw-rw----"), dirs should be 2770
> > ("drwxrws---")).
> >
> > I started setting this up for them in the following manner:
> >
> > mkdir foo.git
> > cd foo.git
> > git init --bare --shared=group
> > cd ..
> > chgrp -R groupname foo.git
> > chmod -R o-rwx foo.git
> >
> > ...and everything looks good, initially...
> >
> > However, when I start pushing into this repo, the newly created files
> > are readable to everybody (files are 0664 ("-rw-rw-r--"), dirs are 2775
> > ("drwxrwsr-x")).
>
> But nobody has access to anything under foo.git since you did
> 'chmod o-rwx foo.git' above.
Yes, it's hard (impossible???) for outside users to get at the files, since
they reside in directories that are not readable to them. However, this does
not at all hide the fact that:
1. The "chmod -R o-rwx" is a command I added myself. Nowhere in Git's
documentation is it said that it is a good idea to run this command.
2. Preferably, when creating a 0660 repo, "git init" should automatically
perform this chmod for you, in the same manner that it already sets the
"set-gid" bit for group-shared repos.
> Unless I'm missing something, I think you already have what you want.
Maybe, but it certainly doesn't fill me with warm, fuzzy, secure feelings.
Am I being overly paranoid?
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] avoid possible overflow in delta size filtering computation
From: Nicolas Pitre @ 2009-03-25 0:39 UTC (permalink / raw)
To: Brandon Casey; +Cc: Junio C Hamano, Kjetil Barvik, git
In-Reply-To: <IJd5MMCs9G_oJF_jS9hZAHkoKM0IvDNHuvHhhQ3MnKbPbSQlMYjOAg@cipher.nrlssc.navy.mil>
On Tue, 24 Mar 2009, Brandon Casey wrote:
> Nicolas Pitre wrote:
> > On a 32-bit system, the maximum possible size for an object is less than
> > 4GB, while 64-bit systems may cope with larger objects. Due to this
> > limitation, variables holding object sizes are using an unsigned long
> > type (32 bits on 32-bit systems, or 64 bits on 64-bit systems).
>
> FYI: except on windows 64-bit where long is still 32 bits AFAIK
Whatever. Same issue.
Nicolas
^ permalink raw reply
* Re: large(25G) repository in git
From: Nicolas Pitre @ 2009-03-25 0:28 UTC (permalink / raw)
To: Adam Heath; +Cc: Sam Hocevar, git
In-Reply-To: <49C95453.9080503@brainfood.com>
On Tue, 24 Mar 2009, Adam Heath wrote:
> Sam Hocevar wrote:
> > In your particular case, I would suggest setting pack.packSizeLimit
> > to something lower. This would reduce the time spent generating a new
> > pack file if the problem were to happen again.
>
> Yeah, saw that one, but *after* I had this problem. The default, if
> not set, is unlimited, which in this case, is definately *not* what we
> want.
In your particular case, if the problem is actually what I think it is,
the pack.packSizeLimit wouldn't have made any difference. This setting
affects local repacking only and has no effect what so ever on the push
operation.
Nicolas
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Jeff Layton @ 2009-03-25 0:24 UTC (permalink / raw)
To: Peter; +Cc: Linus Torvalds, Git Mailing List, Steve French
In-Reply-To: <alpine.LFD.2.00.0903241655210.3032@localhost.localdomain>
On Tue, 24 Mar 2009 17:03:22 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 24 Mar 2009, Peter wrote:
> >
> > Thanks a lot , I will check that out tomorrow, in the meantime, this is the
> > result of your patch being applied:
> >
> > $ git add <big stuff>
> >
> > fatal: error when closing sha1 file (Bad file descriptor)
>
> Ok, that's probably cifs_writepages() doing
>
> open_file = find_writable_file(CIFS_I(mapping->host));
> if (!open_file) {
> cERROR(1, ("No writable handles for inode"));
> rc = -EBADF;
> } else {
> ..
>
> so yeah, looks like it's the fchmod() that triggers it.
>
> I suspect this would be a safer - if slightly slower - way to make sure
> the file is read-only. It's slower, because it is going to look up the
> filename once more, but I bet it is going to avoid this particular CIFS
> bug.
>
Yeah, that should work around it. This CIFS patch should also fix it.
It's untested but it's reasonably simple.
Just out of curiosity, what sort of server are you mounting here?
--------------[snip]------------------
>From ea8dc9927fb9542bb1c73b1982cc44bf3d4a9798 Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@redhat.com>
Date: Tue, 24 Mar 2009 19:50:17 -0400
Subject: [PATCH] cifs: flush data on any setattr
We already flush all the dirty pages for an inode before doing
ATTR_SIZE and ATTR_MTIME changes. There's another problem though -- if
we change the mode so that the file becomes read-only then we may not
be able to write data to it afterward.
Fix this by just going back to flushing all the dirty data on any
setattr call. There are probably some cases that can be optimized out,
but we need to take care that we don't cause a regression by doing that.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/inode.c | 58 ++++++++++++++++++++++++++++--------------------------
1 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index a8797cc..f4e880d 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1792,20 +1792,21 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
goto out;
}
- if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
- /*
- Flush data before changing file size or changing the last
- write time of the file on the server. If the
- flush returns error, store it to report later and continue.
- BB: This should be smarter. Why bother flushing pages that
- will be truncated anyway? Also, should we error out here if
- the flush returns error?
- */
- rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0) {
- cifsInode->write_behind_rc = rc;
- rc = 0;
- }
+ /*
+ * Attempt to flush data before changing attributes. We need to do
+ * this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
+ * ownership or mode then we may also need to do this. Here, we take
+ * the safe way out and just do the flush on all setattr requests. If
+ * the flush returns error, store it to report later and continue.
+ *
+ * BB: This should be smarter. Why bother flushing pages that
+ * will be truncated anyway? Also, should we error out here if
+ * the flush returns error?
+ */
+ rc = filemap_write_and_wait(inode->i_mapping);
+ if (rc != 0) {
+ cifsInode->write_behind_rc = rc;
+ rc = 0;
}
if (attrs->ia_valid & ATTR_SIZE) {
@@ -1903,20 +1904,21 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
return -ENOMEM;
}
- if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
- /*
- Flush data before changing file size or changing the last
- write time of the file on the server. If the
- flush returns error, store it to report later and continue.
- BB: This should be smarter. Why bother flushing pages that
- will be truncated anyway? Also, should we error out here if
- the flush returns error?
- */
- rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0) {
- cifsInode->write_behind_rc = rc;
- rc = 0;
- }
+ /*
+ * Attempt to flush data before changing attributes. We need to do
+ * this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
+ * ownership or mode then we may also need to do this. Here, we take
+ * the safe way out and just do the flush on all setattr requests. If
+ * the flush returns error, store it to report later and continue.
+ *
+ * BB: This should be smarter. Why bother flushing pages that
+ * will be truncated anyway? Also, should we error out here if
+ * the flush returns error?
+ */
+ rc = filemap_write_and_wait(inode->i_mapping);
+ if (rc != 0) {
+ cifsInode->write_behind_rc = rc;
+ rc = 0;
}
if (attrs->ia_valid & ATTR_SIZE) {
--
1.6.0.6
^ permalink raw reply related
* Re: [BUG?] How to make a shared/restricted repo?
From: Brandon Casey @ 2009-03-25 0:26 UTC (permalink / raw)
To: Johan Herland; +Cc: git
In-Reply-To: <200903250105.05808.johan@herland.net>
Johan Herland wrote:
> Hi,
>
> Some colleagues of mine are working on a "secret" project, and they want to
> create a central/server/integration repo that should be group-writable, but
> not at all accessible to anybody outside the group (i.e. files should be
> 0660 ("-rw-rw----"), dirs should be 0770 ("drwxrws---")).
>
> I started setting this up for them in the following manner:
>
> mkdir foo.git
> cd foo.git
> git init --bare --shared=group
> cd ..
> chgrp -R groupname foo.git
> chmod -R o-rwx foo.git
>
> ...and everything looks good, initially...
>
> However, when I start pushing into this repo, the newly created files are
> readable to everybody (files are 0664 ("-rw-rw-r--"), dirs are 0775
> ("drwxrwsr-x")).
But nobody has access to anything under foo.git since you did
'chmod o-rwx foo.git' above.
Unless I'm missing something, I think you already have what you want.
-brandon
^ permalink raw reply
* Re: branch ahead in commits but push claims all up to date
From: John Tapsell @ 2009-03-25 0:26 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Irene Ros, git
In-Reply-To: <alpine.LNX.1.00.0903241304090.19665@iabervon.org>
2009/3/24 Daniel Barkalow <barkalow@iabervon.org>:
> On Tue, 24 Mar 2009, John Tapsell wrote:
>
>> 2009/3/24 Irene Ros <imirene@gmail.com>:
>> > Hi all,
>> >
>> > I've been using git for some time now and haven't run into this issue
>> > before, perhaps someone else here has:
>> >
>> > I have a branch that is ahead of its origin by a few commits:
>> >
>> > $ git status
>> > # On branch myBranch
>> > # Your branch is ahead of 'origin/myBranch' by 10 commits.
>>
>> Tried running: git fetch ?
>>
>> For some weird reason "git push origin mybranch" doesn't actually
>> update origin/mybranch. It's more annoying :-)
>
> It should, so long as you're using the native transport and
> origin/mybranch actually tracks mybranch on origin.
>
> "git push" doesn't update it, but the code that implements the native
> transport does update it if it succeeds.
>
> (Actually, I'm not 100% sure that, if you update origin through some other
> channel with exactly the commit that you now have in mybranch locally, and
> then try the push, it will update the local tracking for that branch; is
> that what you've hit?)
I update via http - maybe that's why? origin/mybranch is never
updated when I push. It's not just a once-off quirk.
John
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Steven French @ 2009-03-25 0:17 UTC (permalink / raw)
To: Jeff Layton; +Cc: Git Mailing List, Linus Torvalds, Peter
In-Reply-To: <20090324193506.0962b28e@tupile.poochiereds.net>
Jeff Layton <jlayton@redhat.com> wrote on 03/24/2009 06:35:06 PM:
> On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> >
> >
> > On Tue, 24 Mar 2009, Peter wrote:
> > > >
> > > > What OS? What filesystem? Are you perhaps running out of space?
> > >
> > > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs
> ( this is a
> > > windows 2000 share mounted with samba in a VMware Workstation
> Debian client (
> > > yes, I know ... )). Memory usage, according to htop, is constant
> = 140/504 MB
> > > during the whole process until git fails.
> >
> > Ok, it sounds very much like a possible CIFS problem.
> >
> > Getting the exact error code for the "close()" will be interesting,
> > because the only thing that can return an error under Linux in close()
is
> > if the filesystem "->flush()" function fails with an error.
> >
> > In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing
the
> > data out, and that in turn calls do_writepages() which in turn calls
> > either generic_writepages() or cifs_writepages() depending on random
cifs
> > crap.
> >
> > I'm not seeing any obvious errors there. But I would _not_ be
surprised if
> > the fchmod(fd, 0444) that we did before the close could be causing
this:
> > cifs gets confused and thinks that it must not write to the file
because
> > the file has been turned read-only.
> >
> > Here's an idea: if this is reproducible for you, does the behavior
change
> > if you do
> >
> > [core]
> > core.fsyncobjectfiles = true
> >
> > in your .git/config file? That causes git to always fsync() the
written
> > data _before_ it does that fchmod(), which in turn means that by the
time
> > the close() rolls around, there should be no data to write, and thus
no
> > possibility that anybody gets confused when there is still dirty data
on a
> > file that has been marked read-only.
> >
> > Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS
go-to
> > guys. It does seem like a CIFS bug is likely.
> >
> > Steve, Jeff: git does basically
> >
> > open(".git/objects/xy/tmp_obj_xyzzy", O_RDWR|O_CREAT|O_EXCL, 0600)
= 5
> > write(5, ".."..., len)
> > fchmod(5, 0444)
> > close(5)
> > link(".git/objects/xy/tmp_obj_xyzzy", ".git/objects/xy/xyzzy");
> > unlink(".git/objects/xy/tmp_obj_xyzzy");
> >
> > to write a new datafile. Under CIFS, that "close()" apparently
sometimes
> > returns with an error, and fails.
> >
> > I guess we could change the "fchmod()" to happen after the close(),
just
> > to make it easier for filesystems to get this right. And yes, as
outlined
> > above, there's a config option to make git use fdatasync() before it
does
> > that fchmod() too. But it does seem like CIFS is just buggy.
> >
> > If CIFS has problems with the above sequence (say, if some timeout
> > refreshes the inode data or causes a re-connect with the server or
> > whatever), then maybe cifs should always do an implicit fdatasync()
when
> > doing fchmod(), just to make sure that the fchmod won't screw up any
> > cached dirty data?
> >
>
> Yes. That's probably the right thing to do here. This is looks like a
> regression that I introduced some time ago in
> cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always
> flushed all the data before doing any setattr. After that delta, we
> just did it on size changes. In a later commit, Steve fixed it so that
> it got done on ATTR_MTIME too.
>
> We can easily change the cifs_setattr variants to flush all the dirty
> data on ATTR_MODE changes, and maybe even change it to only flush on
> certain ATTR_MODE changes, like when we're clearing all the write bits.
>
> I wonder though whether that's sufficient. If we're changing ownership
> for instance, will we hit the same issue? Maybe the safest approach is
> just to go back to flushing on any setattr call.
>
> Steve, thoughts?
Flushing on all setattr calls seems excessive, but the case of
fchmod(0444) followed by close should not require it, and should
work over cifs unless the tcp socket drops between the chmod
and the close/flush (requiring reconnection and reopening
the file handle). For the normal case, the chmod should not
matter, since for cifs, unlike NFSv3, there is a network
open operation and access checks are done on the open call, and
you already have the file open for write). If /proc/fs/cifs/Stats
shows the number of reconnects as 0, then we don't have the case
of dropping the network between chmod and flush/close. Since certain
writebehind errors (in writepages) can not be returned to the application
until flush/close, it is just as likely that what you are seeing is
an error from a previous failed write. The logic for handling
errors in cifs_writepages is much better in recent kernels.
What is the kernel version?
Steve French
Senior Software Engineer
Linux Technology Center - IBM Austin
phone: 512-838-2294
email: sfrench at-sign us dot ibm dot com
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Linus Torvalds @ 2009-03-25 0:11 UTC (permalink / raw)
To: Jeff Layton; +Cc: Peter, Git Mailing List, Steve French
In-Reply-To: <20090324193506.0962b28e@tupile.poochiereds.net>
On Tue, 24 Mar 2009, Jeff Layton wrote:
>
> Yes. That's probably the right thing to do here. This is looks like a
> regression that I introduced some time ago in
> cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always
> flushed all the data before doing any setattr. After that delta, we
> just did it on size changes. In a later commit, Steve fixed it so that
> it got done on ATTR_MTIME too.
Ahh. Yes. You do need to flush on at least ATTR_MODE and ATTR_UID/GID
changes too.
> I wonder though whether that's sufficient. If we're changing ownership
> for instance, will we hit the same issue? Maybe the safest approach is
> just to go back to flushing on any setattr call.
>
> Steve, thoughts?
Is there anything relevant left to be sufficient reason to make it
conditional? Once you're doing it on MODE/SIZE/UID/GID/MTIME changes, that
pretty much will cover all set_attr calls.
Linus
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Linus Torvalds @ 2009-03-25 0:03 UTC (permalink / raw)
To: Peter; +Cc: Git Mailing List, Jeff Layton, Steve French
In-Reply-To: <49C961D0.4010704@mycircuit.org>
On Tue, 24 Mar 2009, Peter wrote:
>
> Thanks a lot , I will check that out tomorrow, in the meantime, this is the
> result of your patch being applied:
>
> $ git add <big stuff>
>
> fatal: error when closing sha1 file (Bad file descriptor)
Ok, that's probably cifs_writepages() doing
open_file = find_writable_file(CIFS_I(mapping->host));
if (!open_file) {
cERROR(1, ("No writable handles for inode"));
rc = -EBADF;
} else {
..
so yeah, looks like it's the fchmod() that triggers it.
I suspect this would be a safer - if slightly slower - way to make sure
the file is read-only. It's slower, because it is going to look up the
filename once more, but I bet it is going to avoid this particular CIFS
bug.
Linus
---
http-push.c | 2 +-
sha1_file.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/http-push.c b/http-push.c
index 48e5f38..ba4fa4d 100644
--- a/http-push.c
+++ b/http-push.c
@@ -748,8 +748,8 @@ static void finish_request(struct transfer_request *request)
aborted = 1;
}
} else if (request->state == RUN_FETCH_LOOSE) {
- fchmod(request->local_fileno, 0444);
close(request->local_fileno); request->local_fileno = -1;
+ chmod(request->tmpfile, 0444);
if (request->curl_result != CURLE_OK &&
request->http_code != 416) {
diff --git a/sha1_file.c b/sha1_file.c
index 4563173..8268da7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2285,9 +2285,8 @@ static void close_sha1_file(int fd)
{
if (fsync_object_files)
fsync_or_die(fd, "sha1 file");
- fchmod(fd, 0444);
if (close(fd) != 0)
- die("unable to write sha1 file");
+ die("error when closing sha1 file (%s)", strerror(errno));
}
/* Size of directory component, including the ending '/' */
@@ -2384,6 +2383,7 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
close_sha1_file(fd);
free(compressed);
+ chmod(tmpfile, 0444);
if (mtime) {
struct utimbuf utb;
utb.actime = mtime;
^ permalink raw reply related
* [BUG?] How to make a shared/restricted repo?
From: Johan Herland @ 2009-03-25 0:05 UTC (permalink / raw)
To: git
Hi,
Some colleagues of mine are working on a "secret" project, and they want to
create a central/server/integration repo that should be group-writable, but
not at all accessible to anybody outside the group (i.e. files should be
0660 ("-rw-rw----"), dirs should be 0770 ("drwxrws---")).
I started setting this up for them in the following manner:
mkdir foo.git
cd foo.git
git init --bare --shared=group
cd ..
chgrp -R groupname foo.git
chmod -R o-rwx foo.git
...and everything looks good, initially...
However, when I start pushing into this repo, the newly created files are
readable to everybody (files are 0664 ("-rw-rw-r--"), dirs are 0775
("drwxrwsr-x")).
Instead of "git init --bare --shared=group", I've tried using
git init --bare --shared=0660
and even
git init --bare &&
git config core.sharedRepository 0660
but the result is still the same.
After reading the "--shared" section in the "git init" man page, this
behaviour is unexpected, and after reading the "core.sharedRepository"
section in the "git config" man page, the current behaviour is IMHO outright
_wrong_. Quoting the "git config" man page:
core.sharedRepository
[...] When 0xxx, where 0xxx is an octal number, files in the repository
will have this mode value. 0xxx will override user’s umask value, and
thus, users with a safe umask (0077) can use this option. [...]
AFAICS, even when I set "core.sharedRepository" to 0660, files are still
created 0664, which is not what the documentation indicates.
Are there other ways to create such shared-but-restricted repositories?
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Documentation: git-format-patch.txt rewordings and cleanups
From: Junio C Hamano @ 2009-03-24 23:55 UTC (permalink / raw)
To: Stephen Boyd; +Cc: J. Bruce Fields, git
In-Reply-To: <780e0a6b0903241636j4749daf3xddb6e4c200c00820@mail.gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> How about a sentence with no negation?
>
> "Note that the leading dot is required if you want a dot between the
> patch name and the suffix."
How about a sentence that does not sound requirement but freedom?
"The leading character does not have to be a dot; for example, you
can use --suffix=-patch to get 0001-description-of-my-change-patch".
^ permalink raw reply
* Re: [PATCH] Documentation: git-format-patch.txt rewordings and cleanups
From: J. Bruce Fields @ 2009-03-24 23:53 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git
In-Reply-To: <780e0a6b0903241636j4749daf3xddb6e4c200c00820@mail.gmail.com>
On Tue, Mar 24, 2009 at 04:36:15PM -0700, Stephen Boyd wrote:
> On Tue, Mar 24, 2009 at 3:09 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Mon, Mar 23, 2009 at 03:21:23AM -0700, Stephen Boyd wrote:
> >> +
> >> -Note that you would need to include the leading dot `.` if you
> >> -want a filename like `0001-description-of-my-change.patch`, and
> >> -the first letter does not have to be a dot. Leaving it empty would
> >> -not add any suffix.
> >> +Note the first letter is not required to be a dot, you will need to
> >> +include the leading dot `.` if you want a filename like
> >> +`0001-description-of-my-change.patch`.
> >
> > That's a comma-splice, trivially fixed by changing "dot, you" to "dot;
> > you".
> >
> > Better?: "Note that the leading dot isn't actually required if you don't
> > want a dot between the patch name and the suffix."
> >
> > (Though personally I'd strike the whole sentence, since a) probably
> > nobody cares, and b) the 1 in a million person that does actually want
> > to do this can figure it out easily enough on their own with a quick
> > test.)
> >
> > --b.
> >
>
> I don't think documentation is meant to point the user to trial and
> error. Even if the error is fairly harmless. Although you could be
> right that nobody cares.
>
> How about a sentence with no negation?
>
> "Note that the leading dot is required if you want a dot between the
> patch name and the suffix."
Sure!
--b.
^ permalink raw reply
* Re: fatal: unable to write sha1 file git 1.6.2.1
From: Jeff Layton @ 2009-03-24 23:35 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Peter, Git Mailing List, Steve French
In-Reply-To: <alpine.LFD.2.00.0903241510010.3032@localhost.localdomain>
On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 24 Mar 2009, Peter wrote:
> > >
> > > What OS? What filesystem? Are you perhaps running out of space?
> >
> > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs ( this is a
> > windows 2000 share mounted with samba in a VMware Workstation Debian client (
> > yes, I know ... )). Memory usage, according to htop, is constant = 140/504 MB
> > during the whole process until git fails.
>
> Ok, it sounds very much like a possible CIFS problem.
>
> Getting the exact error code for the "close()" will be interesting,
> because the only thing that can return an error under Linux in close() is
> if the filesystem "->flush()" function fails with an error.
>
> In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing the
> data out, and that in turn calls do_writepages() which in turn calls
> either generic_writepages() or cifs_writepages() depending on random cifs
> crap.
>
> I'm not seeing any obvious errors there. But I would _not_ be surprised if
> the fchmod(fd, 0444) that we did before the close could be causing this:
> cifs gets confused and thinks that it must not write to the file because
> the file has been turned read-only.
>
> Here's an idea: if this is reproducible for you, does the behavior change
> if you do
>
> [core]
> core.fsyncobjectfiles = true
>
> in your .git/config file? That causes git to always fsync() the written
> data _before_ it does that fchmod(), which in turn means that by the time
> the close() rolls around, there should be no data to write, and thus no
> possibility that anybody gets confused when there is still dirty data on a
> file that has been marked read-only.
>
> Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS go-to
> guys. It does seem like a CIFS bug is likely.
>
> Steve, Jeff: git does basically
>
> open(".git/objects/xy/tmp_obj_xyzzy", O_RDWR|O_CREAT|O_EXCL, 0600) = 5
> write(5, ".."..., len)
> fchmod(5, 0444)
> close(5)
> link(".git/objects/xy/tmp_obj_xyzzy", ".git/objects/xy/xyzzy");
> unlink(".git/objects/xy/tmp_obj_xyzzy");
>
> to write a new datafile. Under CIFS, that "close()" apparently sometimes
> returns with an error, and fails.
>
> I guess we could change the "fchmod()" to happen after the close(), just
> to make it easier for filesystems to get this right. And yes, as outlined
> above, there's a config option to make git use fdatasync() before it does
> that fchmod() too. But it does seem like CIFS is just buggy.
>
> If CIFS has problems with the above sequence (say, if some timeout
> refreshes the inode data or causes a re-connect with the server or
> whatever), then maybe cifs should always do an implicit fdatasync() when
> doing fchmod(), just to make sure that the fchmod won't screw up any
> cached dirty data?
>
Yes. That's probably the right thing to do here. This is looks like a
regression that I introduced some time ago in
cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always
flushed all the data before doing any setattr. After that delta, we
just did it on size changes. In a later commit, Steve fixed it so that
it got done on ATTR_MTIME too.
We can easily change the cifs_setattr variants to flush all the dirty
data on ATTR_MODE changes, and maybe even change it to only flush on
certain ATTR_MODE changes, like when we're clearing all the write bits.
I wonder though whether that's sufficient. If we're changing ownership
for instance, will we hit the same issue? Maybe the safest approach is
just to go back to flushing on any setattr call.
Steve, thoughts?
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply
* Re: [PATCH] Documentation: git-format-patch.txt rewordings and cleanups
From: Stephen Boyd @ 2009-03-24 23:36 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <20090324220913.GN19389@fieldses.org>
On Tue, Mar 24, 2009 at 3:09 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Mar 23, 2009 at 03:21:23AM -0700, Stephen Boyd wrote:
>> +
>> -Note that you would need to include the leading dot `.` if you
>> -want a filename like `0001-description-of-my-change.patch`, and
>> -the first letter does not have to be a dot. Leaving it empty would
>> -not add any suffix.
>> +Note the first letter is not required to be a dot, you will need to
>> +include the leading dot `.` if you want a filename like
>> +`0001-description-of-my-change.patch`.
>
> That's a comma-splice, trivially fixed by changing "dot, you" to "dot;
> you".
>
> Better?: "Note that the leading dot isn't actually required if you don't
> want a dot between the patch name and the suffix."
>
> (Though personally I'd strike the whole sentence, since a) probably
> nobody cares, and b) the 1 in a million person that does actually want
> to do this can figure it out easily enough on their own with a quick
> test.)
>
> --b.
>
I don't think documentation is meant to point the user to trial and
error. Even if the error is fairly harmless. Although you could be
right that nobody cares.
How about a sentence with no negation?
"Note that the leading dot is required if you want a dot between the
patch name and the suffix."
^ 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