* Re: git config -> "fatal: bad config file"
From: David Reitter @ 2009-08-14 14:26 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4A85724C.5060406@drmicha.warpmail.net>
On Aug 14, 2009, at 10:18 AM, Michael J Gruber wrote:
> git needs to read the file because the editor could be configured
> there!
> The only option would be to make git config -e continue past that
> error.
Syntax errors in .git/config could lead to warnings. Since the file is
primarily line-oriented anyways (except for groups), recovery should
be easy.
Also, you could have git-config -e edit a temporary file, check the
file for errors after editing and then move it to .git/config.
^ permalink raw reply
* Re: git config -> "fatal: bad config file"
From: Michael J Gruber @ 2009-08-14 14:18 UTC (permalink / raw)
To: David Reitter; +Cc: git
In-Reply-To: <A5CDBB91-E889-4849-953A-2C1DB4A04513@gmail.com>
David Reitter venit, vidit, dixit 14.08.2009 15:38:
> I made a mistake editing my config file using "git config -e". This
> caused git commands to fail.
>
> Trying to fix the problem, I did
>
> git config -e
> fatal: bad config file line 7 in .git/config
>
> I think the refusal to edit a broken config file is not a good idea.
> It's easy to fix for me of course by editing .git/config directly, but
> git-config should probably not read the config file at all.
>
> Thanks for your consideration.
>
git needs to read the file because the editor could be configured there!
The only option would be to make git config -e continue past that error.
Michael
^ permalink raw reply
* git config -> "fatal: bad config file"
From: David Reitter @ 2009-08-14 13:38 UTC (permalink / raw)
To: git
I made a mistake editing my config file using "git config -e". This
caused git commands to fail.
Trying to fix the problem, I did
git config -e
fatal: bad config file line 7 in .git/config
I think the refusal to edit a broken config file is not a good idea.
It's easy to fix for me of course by editing .git/config directly, but
git-config should probably not read the config file at all.
Thanks for your consideration.
^ permalink raw reply
* trailling whitespace with git am ?
From: PICCA Frédéric-Emmanuel @ 2009-08-14 10:42 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]
hello
I attached a patch that someone sent to me
the public repository is
http://repo.or.cz/w/diffractometer.git
this patch must be apply on the next branch
but when I try to use
git am, I have this sort of message:
mordor% git am /home/picca/Projets/patches/0001-Add-the-POGO-TO-DELETE-comment-to-ComputedAnglesClea.patch
Applying: 'Add the POGO TO DELETE comment to ComputedAnglesClear command'
/tmp/diffractometer/.git/rebase-apply/patch:13: trailing whitespace.
/tmp/diffractometer/.git/rebase-apply/patch:14: trailing whitespace.
/* POGO TO DELETE */
error: patch failed: src/Diffractometer/Diffractometer.cpp:2023
error: src/Diffractometer/Diffractometer.cpp: patch does not apply
Patch failed at 0001 'Add the POGO TO DELETE comment to ComputedAnglesClear command'
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
so git do not want to apply it.
I was looking in the patch file about thoses trailing whitespace but I found nothing
until I rewrote myself this complicate patch :)
then I produced a diff beetween my patch and the original one.
I also attached it.
using emacs/vi I saw immediately the problem... It was a dos/unix file format problem. So the first message of git am was no helpfull at all for me. It would be nice to detect this format
problem when applying the patch and says something like
this patch is in dos format on an unix platform I can not apply it.
the a short explaination of what solution are possibles for the users.
now the question is:
what must I tell to that person if I want to work with her (as she is charming the forget about here is a no op :))
change your editor or is it a git configuration problem ? I suppose she is using msygit but I can not affirm this.
thanks
Frédéric
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-the-POGO-TO-DELETE-comment-to-ComputedAnglesClea.patch --]
[-- Type: text/x-patch; name="0001-Add-the-POGO-TO-DELETE-comment-to-ComputedAnglesClea.patch", Size: 899 bytes --]
From 2ad19d21660e7a94cba0e7eaedbb867208832ca2 Mon Sep 17 00:00:00 2001
From: teresa <tnunez@mail.desy.de>
Date: Fri, 14 Aug 2009 10:46:09 +0200
Subject: [PATCH] 'Add the POGO TO DELETE comment to ComputedAnglesClear command'
---
src/Diffractometer/Diffractometer.cpp | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/Diffractometer/Diffractometer.cpp b/src/Diffractometer/Diffractometer.cpp
index 5d2a55d..98add35 100644
--- a/src/Diffractometer/Diffractometer.cpp
+++ b/src/Diffractometer/Diffractometer.cpp
@@ -2023,6 +2023,8 @@ void Diffractometer::computed_angles_clear()
DEBUG_STREAM << "Diffractometer::computed_angles_clear(): entering... !" << endl;
// Add your own code to control device here
+
+ /* POGO TO DELETE */
_computedAngles_size = 0;
if(attr_ComputedAngles_read) {
delete [] attr_ComputedAngles_read;
--
1.6.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: toto.diff --]
[-- Type: text/x-patch; name="toto.diff", Size: 1881 bytes --]
1,25c1,25
< From 58ed93432f043ee5b2ad44d92a769eddcef4cffa Mon Sep 17 00:00:00 2001
< From: =?utf-8?q?Picca=20Fr=C3=A9d=C3=A9ric-Emmanuel?= <picca@synchrotron-soleil.fr>
< Date: Fri, 14 Aug 2009 12:10:50 +0200
< Subject: [PATCH] * test
<
< ---
< src/Diffractometer/Diffractometer.cpp | 2 ++
< 1 files changed, 2 insertions(+), 0 deletions(-)
<
< diff --git a/src/Diffractometer/Diffractometer.cpp b/src/Diffractometer/Diffractometer.cpp
< index 5d2a55d..98add35 100644
< --- a/src/Diffractometer/Diffractometer.cpp
< +++ b/src/Diffractometer/Diffractometer.cpp
< @@ -2023,6 +2023,8 @@ void Diffractometer::computed_angles_clear()
< DEBUG_STREAM << "Diffractometer::computed_angles_clear(): entering... !" << endl;
<
< // Add your own code to control device here
< +
< + /* POGO TO DELETE */
< _computedAngles_size = 0;
< if(attr_ComputedAngles_read) {
< delete [] attr_ComputedAngles_read;
< --
< 1.6.3.3
<
---
> From 2ad19d21660e7a94cba0e7eaedbb867208832ca2 Mon Sep 17 00:00:00 2001
> From: teresa <tnunez@mail.desy.de>
> Date: Fri, 14 Aug 2009 10:46:09 +0200
> Subject: [PATCH] 'Add the POGO TO DELETE comment to ComputedAnglesClear command'
>
> ---
> src/Diffractometer/Diffractometer.cpp | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/src/Diffractometer/Diffractometer.cpp b/src/Diffractometer/Diffractometer.cpp
> index 5d2a55d..98add35 100644
> --- a/src/Diffractometer/Diffractometer.cpp
> +++ b/src/Diffractometer/Diffractometer.cpp
> @@ -2023,6 +2023,8 @@ void Diffractometer::computed_angles_clear()
> DEBUG_STREAM << "Diffractometer::computed_angles_clear(): entering... !" << endl;
>
> // Add your own code to control device here
> +
> + /* POGO TO DELETE */
> _computedAngles_size = 0;
> if(attr_ComputedAngles_read) {
> delete [] attr_ComputedAngles_read;
> --
> 1.6.4
>
^ permalink raw reply related
* [PATCH] revert: Remove printing fullstop after the commit hash
From: Amit Shah @ 2009-08-14 9:56 UTC (permalink / raw)
To: git; +Cc: Amit Shah
Copy-pasting the commit id from a revert message selects
the trailing fullstop as well which has to be removed
after pasting. That's inefficient.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
builtin-revert.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c
index 151aa6a..7f0c22c 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -346,7 +346,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
add_to_msg(", reversing\nchanges made to ");
add_to_msg(sha1_to_hex(parent->object.sha1));
}
- add_to_msg(".\n");
+ add_to_msg("\n");
} else {
base = parent;
next = commit;
--
1.6.2.5
^ permalink raw reply related
* Re: [PATCH v2] git-cvsimport: add support for cvs pserver password scrambling.
From: Dirk Hörner @ 2009-08-14 9:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Nanako Shiraishi
In-Reply-To: <7vskfusvpq.fsf@alter.siamese.dyndns.org>
Hi Junio,
On Fri, Aug 14, 2009 at 9:25 AM, Junio C Hamano<gitster@pobox.com> wrote:
> Thanks.
>
> While I appreciate your effort to add a test, I'd rather not apply the
> test part of your patch for two reasons:
>
> - It is not a test against a real cvs pserver but is a whitebox test to
> verify that the program says what the program is supposed to spit out
> to the network; and
>
> - It still is a network test that will fail if the TCP port is occupied
> for whatever reason when the test is run, which will make automated
> build and test cycle unreliable.
>
> Unfortunately, I do not see an easy way to run a real cvs pserver
> listening to a local unix domain socket under $TRASH_DIRECTORY, which
> would solve both of the above issues.
>
I agree with you, the tests are not the best. As Dscho stated in one
of the replies to the last version of this patch, the cvs pserver is
quite good to test with because it uses stdin/stdout for
communication. The main problem is git-cvsimport, which right now only
supports tcp sockets.
Ciao,
Dirk
^ permalink raw reply
* Re: [PATCH] Fix "unpack-objects --strict"
From: Frank Lichtenheld @ 2009-08-14 9:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Martin Koegler
In-Reply-To: <7vd46zbjae.fsf@alter.siamese.dyndns.org>
On Thu, Aug 13, 2009 at 12:33:45PM -0700, Junio C Hamano wrote:
> When unpack-objects is run under the --strict option, objects that have
> pointers to other objects are verified for the reachability at the end, by
> calling check_object() on each of them, and letting check_object to walk
> the reachable objects from them using fsck_walk() recursively.
>
> The function however misunderstands the semantics of fsck_walk() function
> when it makes a call to it, setting itself as the callback. fsck_walk()
> expects the callback function to return a non-zero value to signal an
> error (negative value causes an immediate abort, positive value is still
> an error but allows further checks on sibling objects) and return zero to
> signal a success. The function however returned 1 on some non error
> cases, and to cover up this mistake, complained only when fsck_walk() did
> not detect any error.
>
> To fix this double-bug, make the function return zero on all success
> cases, and also check for non-zero return from fsck_walk() for an error.
I've applied this patch and your small follow-up patch here and the error
indeed disappears. I can't comment on the semantical correctness of the
patch.
Thanks,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [EGIT] Push to GitHub caused corruption
From: Robin Rosenberg @ 2009-08-14 9:19 UTC (permalink / raw)
To: John Bito; +Cc: git, Shawn O. Pearce
In-Reply-To: <3ae83b000908111552o53189a44h45fc44f124850483@mail.gmail.com>
onsdag 12 augusti 2009 00:52:29 skrev John Bito <jwbito@gmail.com>:
> Running git fsck -full on the repo that I pushed from is clean.
> Here's the git fsck -full from GitHub
> <mojombo> git fsck --full
> <mojombo> broken link from tree
> f4f9ecd1875938baa42467dfd6a8134d75fe5de4 to tree
> 57548924f1eca854dc8db00844f95d3de2c82957
> <mojombo> broken link from tree
> f4f9ecd1875938baa42467dfd6a8134d75fe5de4 to tree
> 3d1f74522c3e7c3c03390fae376446fda6eed306
> <mojombo> missing tree 3d1f74522c3e7c3c03390fae376446fda6eed306
> <mojombo> missing tree 57548924f1eca854dc8db00844f95d3de2c82957
> <mojombo> dangling commit ab6ce47159c1eaff0e4bae19291679267de9f669
>
> The repo on GitHub is back where it was before the push. I have a
> copy of the corrupt one from GitHub (358MB tar.gz). If there's
> something I can do that would help to improve JGit/EGit, please let me
> know.
I guess we'd like to try to recreate the problem, unless we see some obvious clue. What
does the tres with missing links look like, i.e. f4f9ecd1875938baa42467dfd6a8134d75fe5de4.
On the recreation side we'd probably need to recreate a repo that looks somewhat like the
github repo before the crash. We can do that by taking a copy of your existing (good) repo,
reset branches and clone it. Then push from yet another copy and hopefully it will blow up
again.
Can we be sure it wasn't corrupt before the jgit push?
Did f4f9ecd1875938baa42467dfd6a8134d75fe5de4 exist before the push or was it new?
-- robin
^ permalink raw reply
* Re: [RFC PATCH 0/2] add a --delete option to git push
From: Jakub Narebski @ 2009-08-14 8:50 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, Git List
In-Reply-To: <1250226349-20397-1-git-send-email-srabbelier@gmail.com>
On Fri, 14 August 2009, Sverre Rabbelier wrote:
> This is in reply to a brief conversation I had with jnareb on #git
> about the blogpost "5 things git could learn from hg", or something
> like that (can't find it right now).
Thanks.
It was "Five Features from Mercurial That Would Make Git Suck Less"
http://nubyonrails.com/articles/five-features-from-mercurial-that-would-make-git-suck-less
Those five features were:
1. "<scm> init <directory>" (done)
2. "hg commit --close-branch" vs slightly cryptic
"git push origin :refs/heads/feature-tweak", which can be written
simply as "git push origin :feature-tweak" I think.
(that is what this patch series is about)
3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
but more realistic example would be "hg checkout 6324" :-P
4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
5. "hg serve" (gitweb and a kind of git-daemon equivalent)
See the alleged blog for details (I call it 'alleged blog' because real
blog has comments which work, and are shown soon after posting them).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Work around performance bug in subprocess.Popen.communicate()
From: Karl Wiberg @ 2009-08-14 8:12 UTC (permalink / raw)
To: Erik Sandberg; +Cc: Catalin Marinas, git
In-Reply-To: <e87cdfda0908140026x21b8aad3x82417d4b1c8016f4@mail.gmail.com>
On 2009-08-14 09:26:06 +0200, Erik Sandberg wrote:
> Wouldn't it be better to only use the write() workaround when
> sys.version indicates that the Python bug may be present, and use
> communicate() properly otherwise?
In my opinion: no. The workaround has no negative effect (since we
know that git is well-behaved). Making it conditional would only make
the code more complex, without gaining us anything.
--
Karl Wiberg, Virtutech
^ permalink raw reply
* Re: merging individual files
From: Michael J Gruber @ 2009-08-14 7:55 UTC (permalink / raw)
To: Chris Marshall; +Cc: git
In-Reply-To: <loom.20090813T192334-7@post.gmane.org>
Chris Marshall venit, vidit, dixit 13.08.2009 22:16:
> Suppose that merging branch dev1 into master would result in three files, f1,
> f2, and f3 being changed, and that I only want to merge the changes for f1 and
> f2 and not the changes for f3 currently. Later on, I want to accept the f3
> changes. Suppose further that the changes to f1, f2, and f3 occurred in a
> single commit to branch dev1.
>
> What is the simplest way to use git to achieve that effect?
>
> More generally, I need a way to accept the changes for one or two files while
> rejecting the changes for a potentially large number of files, then later on
> accepting the changes for the large number of files.
>
> I work at a company where perforce is currently used for all development and am
> trying to work out the git equivalents to all of the perforce flows we use.
> This workflow is the only one that I am stumped on.
>
> One solution that occurs to me is to create a temporary branch off of the (most
> recent) common ancestor of master and br1, let's say br2, checkout the files
> from br1 that I want to merge into master and commit those to br2, then merge
> br2 into master:
>
> git checkout common_ancestor_commit
> git checkout -b br2
> git checkout br1 f1 f2
> git commit
> git checkout master
> git merge br2
> git branch -d br2
>
> This strikes me as not too bad of a procedure, as long as there is a graceful
> way of determining the most recent common ancestor of br1 and master. What's
> the simplest way of doing that?
>
That would be simply git merge-base master br1.
BUT: The main problem here is that git is not file based, but
revision/commit/tree based. In the above, you're basically losing all
the history common_ancestor_commit..br1 which produced br1's version of
f1 and f2, in the sense that a git log master will not show that part of
the history at all.
If it makes sense to change f1 and f2 without changing f3 that probably
means that the pertinent commit should have been split. Is it an option
for you to rewrite br1's history? That would be the most gittish solution.
Alternatively, you could
git checkout master
git merge --no-commit br1
git checkout --ours f3
git commit # add a note about f3 in the commit message
That way at least you have the full history on master. Note that this
will tell git that br1 is merged in, that is you can't use merge later
on to pull in the changes to f3.
Then, when you want to get that change to f3, you can br1's version of
f3 using git checkout br1 -- f3 and committing that to master. But you
should really rewrite br1 ;)
Michael
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Junio C Hamano @ 2009-08-14 7:51 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Git List
In-Reply-To: <7vab22ubr5.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I do not mean that this new feature is useless nor stupid. Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally. There are branches on the other side,
> and there is a special operation called 'delete' that you can inflict on
> them.
>
> But it is a different mental model of how git natively does "push".
> ...
> These are both valid models. They just do not mix, so let's avoid
> confusion by not allowing both at the same time.
One more thing. I suspect that adding --delete and nothing else probably
makes things worse than not doing anything.
In a mental model where "push there --delete $branch" is natural, there
are branches on the other side, and when you run 'push' command, you name
the special operation, 'delete', that you would want to inflict on them.
In such a world, there probably are other (perhaps not so special)
operations you can inflict on the branches on the other side as well.
They are probably called something like:
push there --create $branch $commit
push there --update $branch $commit
If you give them only --delete without completing the vocabulary by giving
these operations as well, you would force people to mix "git native" world
model (i.e. there is no "mode" nor "opration"; there is only "list of
instructions, each of which encodes the equivalent of 'mode'") with this
Hg-inspired world model.
^ permalink raw reply
* Re: [PATCH] Fix "unpack-objects --strict"
From: Junio C Hamano @ 2009-08-14 7:41 UTC (permalink / raw)
To: Martin Koegler; +Cc: Frank Lichtenheld, git
In-Reply-To: <20090814060307.GA31721@auto.tuwien.ac.at>
Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
> What about this check:
>> @@ -184,7 +184,7 @@ static int check_object(struct object *obj, int type, void *data)
>> if (!obj)
>> return 0;
>
> This is neccessary to skip already written objects (eg. blobs,
> obj_list[i].obj == NULL).
You can fix that issue by teaching write_rest() to check what it feeds
check_object(), can't you?
> I'm not sure, if fsck_walk can call check_object with obj == NULL
> under some (rare) conditions. If yes, the return code should be
> changed to 1.
I think that is a sensible change to signal an error regardless. For
example, fsck_walk_tree() will make a callback to you (meaning, walk()
function pointer points at your check_object() function) like this:
while (tree_entry(&desc, &entry)) {
int result;
if (S_ISGITLINK(entry.mode))
continue;
if (S_ISDIR(entry.mode))
result = walk(&lookup_tree(entry.sha1)->object, OBJ_TREE, data);
so while you are checking a tree object you received, upon hitting a
subtree of that tree, it will lookup_tree() it, and if that tree is
missing, you will be called with NULL.
On top of the previous patch, a fix would look like this, I think, but
please double check.
Thanks.
builtin-unpack-objects.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 2522c2d..bae00ea 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -181,7 +181,7 @@ static void write_cached_object(struct object *obj)
static int check_object(struct object *obj, int type, void *data)
{
if (!obj)
- return 0;
+ return 1;
if (obj->flags & FLAG_WRITTEN)
return 0;
@@ -209,8 +209,10 @@ static int check_object(struct object *obj, int type, void *data)
static void write_rest(void)
{
unsigned i;
- for (i = 0; i < nr_objects; i++)
- check_object(obj_list[i].obj, OBJ_ANY, 0);
+ for (i = 0; i < nr_objects; i++) {
+ if (obj_list[i].obj)
+ check_object(obj_list[i].obj, OBJ_ANY, 0);
+ }
}
static void added_object(unsigned nr, enum object_type type,
^ permalink raw reply related
* Re: [PATCH] Fix "unpack-objects --strict"
From: Junio C Hamano @ 2009-08-14 7:31 UTC (permalink / raw)
To: Martin Koegler; +Cc: Frank Lichtenheld, git
In-Reply-To: <20090814071949.GA2342@auto.tuwien.ac.at>
Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
> I'm talking on the server side of a push operation (receive-pack), not
> the client side. The patchset should prevent invalid data from
> entering the repository, thereby preventing upload-pack (during further
> fetch operation) and other git programs (eg. called from gitweb) from
> failing/segfaulting.
upload-pack won't feed starting from a random object for a reason, so your
worry is unfounded.
I'd agree gitweb may be a problem. Ideally it should restrict itself from
anything reachable from the refs of he repository---patches welcome.
^ permalink raw reply
* Re: [PATCH] Work around performance bug in subprocess.Popen.communicate()
From: Erik Sandberg @ 2009-08-14 7:26 UTC (permalink / raw)
To: Karl Wiberg; +Cc: Catalin Marinas, git
In-Reply-To: <20090814062149.GA23179@lux.e.vtech>
On Fri, Aug 14, 2009 at 8:21 AM, Karl Wiberg<kha@virtutech.com> wrote:
> On 2009-08-13 23:18:59 +0100, Catalin Marinas wrote:
>
>> Since we are only using Git, I'll merge this patch
>> (and maybe add a comment).
>
> Sure.
Wouldn't it be better to only use the write() workaround when
sys.version indicates that the Python bug may be present, and use
communicate() properly otherwise?
Erik
^ permalink raw reply
* Re: [PATCH v2] git-cvsimport: add support for cvs pserver password scrambling.
From: Junio C Hamano @ 2009-08-14 7:25 UTC (permalink / raw)
To: Dirk Hoerner; +Cc: git, Johannes Schindelin, Nanako Shiraishi
In-Reply-To: <1250233111-19188-1-git-send-email-dirker@gmail.com>
Dirk Hoerner <dirker@gmail.com> writes:
> Instead of a cleartext password, the CVS pserver expects a scrambled one
> in the authentication request. With this patch it is possible to import
> CVS repositories only accessible via pserver and user/password.
>
> Signed-off-by: Dirk Hoerner <dirker@gmail.com>
Thanks.
While I appreciate your effort to add a test, I'd rather not apply the
test part of your patch for two reasons:
- It is not a test against a real cvs pserver but is a whitebox test to
verify that the program says what the program is supposed to spit out
to the network; and
- It still is a network test that will fail if the TCP port is occupied
for whatever reason when the test is run, which will make automated
build and test cycle unreliable.
Unfortunately, I do not see an easy way to run a real cvs pserver
listening to a local unix domain socket under $TRASH_DIRECTORY, which
would solve both of the above issues.
^ permalink raw reply
* Re: [PATCH] Fix "unpack-objects --strict"
From: Martin Koegler @ 2009-08-14 7:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Frank Lichtenheld, git
In-Reply-To: <7vocqiucpw.fsf@alter.siamese.dyndns.org>
On Thu, Aug 13, 2009 at 11:32:59PM -0700, Junio C Hamano wrote:
> Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
> > To introduce a object with one of its linked objects missing, left it
> > out of the pack and push it into the repository. unpack-objects will
> > unpack all objects and fail updating the ref (but leave all objects in
> > the repository). As second step, simply send a ref update request,
> > which should succed, as the object is present in the repository.
>
> Your "ref update request" exploit does not work because your understanding
> of how we decide to allow updating a ref is flawed.
>
> We do not blindly update a ref to a commit only because we happen to have
> that commit. We require that commit to reach existing tips of refs
> without break. The logic is in quickfetch() in builtin-fetch.c.
I'm talking on the server side of a push operation (receive-pack), not
the client side. The patchset should prevent invalid data from
entering the repository, thereby preventing upload-pack (during further
fetch operation) and other git programs (eg. called from gitweb) from
failing/segfaulting.
mfg Martin Kögler
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Sverre Rabbelier @ 2009-08-14 7:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Git List
In-Reply-To: <7vab22ubr5.fsf@alter.siamese.dyndns.org>
Heya,
On Thu, Aug 13, 2009 at 23:53, Junio C Hamano<gitster@pobox.com> wrote:
> I doubt that assumption is warranted. I've seen new people on this list
> who want to be as specific as possible before they get familiar with the
> tool (I guess it is in the same spirit that they like to spell out long
> option names instead of short ones).
Fair enough.
> Your "old:new" demonstrates a fuzzy understanding of the underlying
> concept. They are not <old> nor <new>. They are <object name> and
> <destination>; with this object, update that destination.
Not really, I was responding to Jeff's "you can rename a branch like
this", I know that in <left>:<right> the <left> can be anything that
locally resolves to an object, and that <right> can be any valid ref
on the remote side, but that's quite a mouthful :P.
> Also --delete should imply not
> looking at configured refspecs at all. After all, this is incompatible
> with the way git expresses push with refspecs, and trying to mix these two
> would lead to confusion.
Agreed, I don't think "git push --delete origin master:master
to-be-deleted" is a good idea.
> [...] Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally. [...]
>
> [...] In
> git model, you give list of instructions <which branch to update with what
> commit>, and as a special case "what commit" could be "empty" to signal
> deletion, and "push" carries out the instructions.
>
> These are both valid models. They just do not mix, so let's avoid
> confusion by not allowing both at the same time.
Agreed, would it be enough to ensure that there are refs present (as
argument), and
that they do not contain a colon?
Also, how do I go about making sure the local configuration is
ignored, as you mentioned above?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Jeff King @ 2009-08-14 7:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Jakub Narebski, Git List
In-Reply-To: <20090814070021.GB7113@coredump.intra.peff.net>
On Fri, Aug 14, 2009 at 03:00:21AM -0400, Jeff King wrote:
> Thank you for succintly explaining what I was trying to put my finger on
> elsewhere in the thread. To sum up what I was trying to say:
>
> I don't have anything against the "delete mode" mental model, but I
> don't have a particular use for it. My counter-proposal was "syntactic
> sugar without changing the mental model". Between the two, I don't have
> a strong feeling (and my "this is wrong" comments were about where you
> had mixed the two, and I think you agreed with them).
To clarify: "you" in the first paragraph is Junio, and in the second it
is Sverre. :)
-Peff
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Jeff King @ 2009-08-14 7:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Jakub Narebski, Git List
In-Reply-To: <7vab22ubr5.fsf@alter.siamese.dyndns.org>
On Thu, Aug 13, 2009 at 11:53:50PM -0700, Junio C Hamano wrote:
> I do not mean that this new feature is useless nor stupid. Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally. There are branches on the other side,
> and there is a special operation called 'delete' that you can inflict on
> them.
>
> But it is a different mental model of how git natively does "push". In
> git model, you give list of instructions <which branch to update with what
> commit>, and as a special case "what commit" could be "empty" to signal
> deletion, and "push" carries out the instructions.
>
> These are both valid models. They just do not mix, so let's avoid
> confusion by not allowing both at the same time.
Thank you for succintly explaining what I was trying to put my finger on
elsewhere in the thread. To sum up what I was trying to say:
I don't have anything against the "delete mode" mental model, but I
don't have a particular use for it. My counter-proposal was "syntactic
sugar without changing the mental model". Between the two, I don't have
a strong feeling (and my "this is wrong" comments were about where you
had mixed the two, and I think you agreed with them).
-Peff
^ permalink raw reply
* [PATCH v2] git-cvsimport: add support for cvs pserver password scrambling.
From: Dirk Hoerner @ 2009-08-14 6:58 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Junio C Hamano, Nanako Shiraishi,
Dirk Hoerner
Instead of a cleartext password, the CVS pserver expects a scrambled one
in the authentication request. With this patch it is possible to import
CVS repositories only accessible via pserver and user/password.
Signed-off-by: Dirk Hoerner <dirker@gmail.com>
---
git-cvsimport.perl | 39 ++++++++++++++++++++++++++++++++++++++-
t/t9600-cvsimport.sh | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e439202..593832d 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -252,7 +252,8 @@ sub conn {
}
};
}
- $pass="A" unless $pass;
+
+ $pass = $self->_scramble($pass);
my ($s, $rep);
if ($proxyhost) {
@@ -484,6 +485,42 @@ sub _fetchfile {
return $res;
}
+sub _scramble {
+ my ($self, $pass) = @_;
+ my $scrambled = "A";
+
+ return $scrambled unless $pass;
+
+ my $pass_len = length($pass);
+ my @pass_arr = split("", $pass);
+ my $i;
+
+ # from cvs/src/scramble.c
+ my @shifts = (
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+ 114,120, 53, 79, 96,109, 72,108, 70, 64, 76, 67,116, 74, 68, 87,
+ 111, 52, 75,119, 49, 34, 82, 81, 95, 65,112, 86,118,110,122,105,
+ 41, 57, 83, 43, 46,102, 40, 89, 38,103, 45, 50, 42,123, 91, 35,
+ 125, 55, 54, 66,124,126, 59, 47, 92, 71,115, 78, 88,107,106, 56,
+ 36,121,117,104,101,100, 69, 73, 99, 63, 94, 93, 39, 37, 61, 48,
+ 58,113, 32, 90, 44, 98, 60, 51, 33, 97, 62, 77, 84, 80, 85,223,
+ 225,216,187,166,229,189,222,188,141,249,148,200,184,136,248,190,
+ 199,170,181,204,138,232,218,183,255,234,220,247,213,203,226,193,
+ 174,172,228,252,217,201,131,230,197,211,145,238,161,179,160,212,
+ 207,221,254,173,202,146,224,151,140,196,205,130,135,133,143,246,
+ 192,159,244,239,185,168,215,144,139,165,180,157,147,186,214,176,
+ 227,231,219,169,175,156,206,198,129,164,150,210,154,177,134,127,
+ 182,128,158,208,162,132,167,209,149,241,153,251,237,236,171,195,
+ 243,233,253,240,194,250,191,155,142,137,245,235,163,242,178,152
+ );
+
+ for ($i = 0; $i < $pass_len; $i++) {
+ $scrambled .= pack("C", $shifts[ord($pass_arr[$i])]);
+ }
+
+ return $scrambled;
+}
package main;
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 363345f..57c0eac 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -128,4 +128,45 @@ test_expect_success 'import from a CVS working tree' '
test_expect_success 'test entire HEAD' 'test_cmp_branch_tree master'
+if ! type nc >/dev/null 2>&1
+then
+ say 'skipping cvsimport pserver test, nc not found'
+ test_done
+ exit
+fi
+
+cat << EOF >expected
+BEGIN AUTH REQUEST
+/cvs
+me
+AyuhedEIc?^]'%=0:q Z,b<3!a>
+END AUTH REQUEST
+EOF
+
+test_expect_success 'connect to pserver with password' '
+
+ echo "I HATE YOU" | nc -l 2401 >actual &
+ test_must_fail git cvsimport -d \
+ :pserver:me:abcdefghijklmnopqrstuvwxyz@localhost:/cvs foo \
+ >/dev/null 2>&1 &&
+ test_cmp expected actual
+'
+
+cat << EOF >expected
+BEGIN AUTH REQUEST
+/cvs
+anonymous
+A
+END AUTH REQUEST
+EOF
+
+test_expect_success 'connect to pserver without password' '
+
+ echo "I HATE YOU" | nc -l 2401 >actual &
+ test_must_fail git cvsimport -d \
+ :pserver:anonymous@localhost:/cvs foo \
+ >/dev/null 2>&1 &&
+ test_cmp expected actual
+'
+
test_done
--
1.6.4
^ permalink raw reply related
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Jeff King @ 2009-08-14 6:55 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Junio C Hamano, Git List
In-Reply-To: <fabb9a1e0908132340n10da3e38kfab07ab2cff18c82@mail.gmail.com>
On Thu, Aug 13, 2009 at 11:40:44PM -0700, Sverre Rabbelier wrote:
> > On the other hand, "--delete <ref>" introduces its own syntactic
> > problems. [...]
>
> It does indeed, and I don't think that's the way to go.
Hmm. Actually, looking at the code, we _already_ have a funny
two-element syntax:
git push origin tag v1.6.1
which, AFAICT, is totally useless, as you can just push v1.6.1 directly.
I assume it's historical. I still think it's probably not a good idea to
introduce a similar "delete foo".
> > Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
> > a reasonable way of adding just the syntactic sugar. [...]
>
> That would work too I guess, although it would be technically more difficult.
Really? I was thinking something as simple as:
diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..aa3784c 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -44,6 +44,12 @@ static void set_refspecs(const char **refs, int nr)
strcat(tag, refs[i]);
ref = tag;
}
+ if (!prefixcmp("--delete=", ref)) {
+ struct strbuf deleted = STRBUF_INIT;
+ strbuf_addstr(&deleted, ":");
+ strbuf_addstr(&deleted, skip_prefix(ref, "--delete="));
+ ref = strbuf_detach(&deleted, NULL);
+ }
add_refspec(ref);
}
}
which is even shorter than your patch, not needing a separate option
parser.
That being said, currently parseopt will complain about that, even after
the "remote" field; we would need to pass it
PARSE_OPT_STOP_AT_NON_OPTION.
> I don't think it's that confusing either, but it's hard to stumble
> upon, yes? When you're looking at the man page for git push it is
> easier to deduct that '--delete' is what you need, than ':master'.
I think you mean "deduce", but yes, I think we have seen people complain
about the syntax in the past. I'm not against fixing it; I just want to
make sure what we introduce doesn't make any new confusion.
-Peff
^ permalink raw reply related
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Junio C Hamano @ 2009-08-14 6:53 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Git List
In-Reply-To: <1250226349-20397-2-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> Currently `git push --delete master:master` results in a somewhat
> cryptic error message. ...
What does "git push --delete name:destination" say?
What does "git push --delete :destination" say?
> ... It seems unlikely however, that those new
> to git would use the 'old:new' notation,...
I doubt that assumption is warranted. I've seen new people on this list
who want to be as specific as possible before they get familiar with the
tool (I guess it is in the same spirit that they like to spell out long
option names instead of short ones).
And I happen to think it is a good discipline, when learning a new tool,
to understand the underlying generic model before advancing to lazy and
useful short-hand. That's how I teach in my upcoming book.
Your "old:new" demonstrates a fuzzy understanding of the underlying
concept. They are not <old> nor <new>. They are <object name> and
<destination>; with this object, update that destination. And you can
abbreviate when they are textually spelled the same. I.e. "git push
origin master" is equivalent to "git push origin master:master" because
both sides are spelled 'm a s t e r' the same way.
Having said all that.
I tend to agree with Jeff that it would probably make sense to limit this
new feature to colonless form and error out if you see a refspec with a
colon and --delete at the same time. Also --delete should imply not
looking at configured refspecs at all. After all, this is incompatible
with the way git expresses push with refspecs, and trying to mix these two
would lead to confusion.
I do not mean that this new feature is useless nor stupid. Being able to
say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
Hg inspired one) _very_ naturally. There are branches on the other side,
and there is a special operation called 'delete' that you can inflict on
them.
But it is a different mental model of how git natively does "push". In
git model, you give list of instructions <which branch to update with what
commit>, and as a special case "what commit" could be "empty" to signal
deletion, and "push" carries out the instructions.
These are both valid models. They just do not mix, so let's avoid
confusion by not allowing both at the same time.
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Sverre Rabbelier @ 2009-08-14 6:40 UTC (permalink / raw)
To: Jeff King; +Cc: Jakub Narebski, Junio C Hamano, Git List
In-Reply-To: <20090814063359.GA6898@coredump.intra.peff.net>
Heya,
On Thu, Aug 13, 2009 at 23:33, Jeff King<peff@peff.net> wrote:
> On Thu, Aug 13, 2009 at 11:24:05PM -0700, Sverre Rabbelier wrote:
>> I don't think we should touch any configured refspecs, think about how
>> often one would use that vs. the inconvenience of doing so
>> unintentionally.
>
> I think you are right. My previous message was sort of thinking out
> loud, but I think on the whole, the annoyance caused by accidental
> deletion is not worth it. :)
Thinking out loud is good :).
> I guess I find what you are doing _more_ complex, because you are really
> introducing a whole new mode to push, which is "I am deleting some
> stuff". As opposed to some syntactic sugar to replace the confusing
> ":ref" syntax, which is what I thought the goal was.
Yes, replacing the confusing ":ref" syntax was the goal, but I think
the current solution does that as well.
> On the other hand, "--delete <ref>" introduces its own syntactic
> problems. [...]
It does indeed, and I don't think that's the way to go.
> Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
> a reasonable way of adding just the syntactic sugar. [...]
That would work too I guess, although it would be technically more difficult.
> Of course, maybe the goal of a "delete mode" is useful to people. I
> can't think of a time when I would have used it, but then I also tend to
> think ":<ref>" is elegant and obvious. ;)
I don't think it's that confusing either, but it's hard to stumble
upon, yes? When you're looking at the man page for git push it is
easier to deduct that '--delete' is what you need, than ':master'.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Jeff King @ 2009-08-14 6:33 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Junio C Hamano, Git List
In-Reply-To: <fabb9a1e0908132324td6869aydc752f67b95546f1@mail.gmail.com>
On Thu, Aug 13, 2009 at 11:24:05PM -0700, Sverre Rabbelier wrote:
> > It seems like it would be simple enough to just check whether the
> > refspec contains a colon; if so, silently leave it alone. That could
> > also protect configured refspecs, as mentioned above, but I wouldn't
> > rule out somebody have a single-name refspec in their config (in fact, I
> > think "remote.$X.push = HEAD" is reasonable -- should that delete the
> > HEAD on "git push --delete"?).
>
> I don't think we should touch any configured refspecs, think about how
> often one would use that vs. the inconvenience of doing so
> unintentionally.
I think you are right. My previous message was sort of thinking out
loud, but I think on the whole, the annoyance caused by accidental
deletion is not worth it. :)
> > So maybe it would make more sense for it to be "--delete <ref>" and
> > impact only a single ref. The simple case of "git push --delete foo"
> > would remain unchanged.
>
> I thought about that, but I decided that it was both intuitive and
> convenient to be able to delete multiple refs this way.
> [...]
> I do, it's easy to make mistakes when it's more powerful, and I think
> less intuitive. I think we want this to be as intuitive as possible.
I guess I find what you are doing _more_ complex, because you are really
introducing a whole new mode to push, which is "I am deleting some
stuff". As opposed to some syntactic sugar to replace the confusing
":ref" syntax, which is what I thought the goal was.
On the other hand, "--delete <ref>" introduces its own syntactic
problems. Is it an option, in which case you end up doing:
git push --delete master origin
which is a bit backwards from the usual syntax. Or is it part of the
refspec list, in which case:
1. We have just disallowed a refspec called "--delete" (though to be
fair, you have to be a little insane to use that anyway, and you
can always call it refs/heads/--delete)).
2. Now we don't simply have a list, one refspec per element, which
makes things syntactically a little more complex.
Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
a reasonable way of adding just the syntactic sugar. I.e.:
git push origin --delete=master
Of course, maybe the goal of a "delete mode" is useful to people. I
can't think of a time when I would have used it, but then I also tend to
think ":<ref>" is elegant and obvious. ;)
I dunno. I don't feel too strongly about it; mainly I was just surprised
because I would have done it the other way. :)
-Peff
^ 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