Git development
 help / color / mirror / Atom feed
* Re: metastore
From: Josh England @ 2007-09-17 19:46 UTC (permalink / raw)
  To: david
  Cc: martin f krafft, git, Daniel Barkalow, Junio C Hamano,
	Johannes Schindelin, Thomas Harning Jr., Francis Moreau,
	Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709171011160.1558@asgard.lang.hm>

I'd like to point out the following two posts, as I think they are
relevant to this thread:

[PATCH] example hook script to save/restore file permissions/ownership
http://marc.info/?l=git&m=118953004817642&w=2

[PATCH] post_merge hook, related documentation, and tests
http://marc.info/?l=git&m=118953004730496&w=2

The hook script above runs in a pre-commit hook to write out file
metadata to a file in the repository.  It can then be run from the
post-merge hook (patch above) to restore permissions.  Running it from a
post-checkout hook may be more appropriate, but post-merge seems to work
well for my purposes.  The script handles merge conflicts and (in my
testing) does the right thing.  I'm using it now to track metadata for
not just /etc, but an entire linux image.

It will handle merge conflicts by recognizing that the metadata file had
a conflict, and will direct the user to resolve the conflict and reset
working dir perms before allowing a commit.

-JE

^ permalink raw reply

* Re: metastore
From: Junio C Hamano @ 2007-09-17 19:19 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: david, Johannes Schindelin, martin f krafft, git,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709171308150.5298@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> .... Things can be a lot more 
> acceptable if the intrusive changes are improvements for the 
> maintainability of the normal case, and the special case code is no longer 
> intrusive at all.

Very well said.

^ permalink raw reply

* Re: Git pull fails on a repository > 1.5G.
From: Junio C Hamano @ 2007-09-17 19:18 UTC (permalink / raw)
  To: pradeep singh; +Cc: git, Dhirendra Singh Kholia
In-Reply-To: <a901b49a0709170239t39123a27t3ff3462855c7eb57@mail.gmail.com>

"pradeep singh" <pradeep.rautela@gmail.com> writes:

>> The remote repo has some changes to a file with some simple printk's
>> some some code changes.
>>
>> I have my git repo in /mnt/reiser/project .
>>
>> I changed to the my repo.

"changed" meaning you did:

          $ cd /mnt/reiser/project

right?

>> i did a git-pull ssh://user1@10.100.205.34/opt/test/project test .[to
>> pull from another test machine].

It is not clear if this "another test machine" is the "remote
repo with printk" you earlier talked about.  Are they the same?
I am assuming they are.  What was the command?  Was

	$ git-pull ssh://user1@10.100.205.34/opt/test/project test .

what you typed (I am hoping that you did not give the extra " ."
at the end)?

>> I got some conflicts in a file but in some important files it
>> did not update it.

You earlier talked about "printk's to a file" in the remote, and
then you talk about two other classes of files.  How are they
related?  Is the "files it did not update it" being "important"
a helpful information to diagnose this issue?

Getting conflicts is perfectly normal.  If you changed something
and the branch you are pulling from changed the area nearby, you
would get conflicts and need to resolve.  Are you saying the
file you got conflict in should not have get conflicts?  If so,
what makes you think so?  They did not see parallel developments
while they diverged?

It also is perfectly normal if "git-pull" does not rewrite every
file in your work tree.  If a file was modified by you (in the
repository /mnt/reiser/project) since the history diverged from
the history "another test machine" has, while the repository on
that another test machine did not modify that file in any way,
the merge result is _your_ version and in such a case "git-pull"
will keep your copy intact.

What made you think that some files that were not updated should
have been updated?  IOW, are you sure they should have been
updated while git did not update them?  What error messages did
you get when you did the above "pull"?  What did they say?

> What is happening here? :-/
>
> I am puzzled.
> Can anybody please help me???

Sorry, you provided too little information to start even
speculating if there is anything wrong going on, let alone
diagnosing if there indeed is something wrong.

^ permalink raw reply

* Re: RFC: German translation vocabulary
From: Christian Stimming @ 2007-09-17 19:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alexander Wuerstlein, David Kastrup, git
In-Reply-To: <Pine.LNX.4.64.0709171013230.28586@racer.site>

Am Montag, 17. September 2007 11:17 schrieb Johannes Schindelin:
> > > >> msgid "commit [noun]"
> > > >> msgstr "?bertragung (Sendung?, ?bergabe?, Einspielung?,
> > > >> Ablagevorgang?)"
> > > >
> > > > "Vorgang"? (think Beamtendeutsch)
> > >
> > > Buchung, Einbuchung, Verbuchung, Registrierung?
> >
> > Transaktion?
>
> The real problem is that we use "commit" in two senses:
>
> - the action ("to commit", but also, "to do a commit") of making a new
>   revision, but also
>
> - the revision in the revision graph ("is this in the commit abcdef?").

This is exactly the noun vs. the verb ambiguity. This is exactly why those are 
mentioned as two different entries in the glossary.

> So I do not think that any proposals reflect the ambiguity of "a commit".
> I actually talk about "Revision" in German, when I refer to a commit.

Yes, that's what I am leaning towards as well. Let's see.

Christian

^ permalink raw reply

* [EGIT PATCH] Add feature and plugin.
From: Ben Konrath @ 2007-09-17 18:53 UTC (permalink / raw)
  To: git

Hi,

I made a feature and associated branding plugin for Egit. Including
these two plugins allows us to build Egit for Fedora but it also makes
it easy to create an update site for Egit. These two plugins also add an
entry for Egit in Help -> About Eclipse -> Feature Details. 

This is an updated version of the patch that I originally sent privately to 
Robin Rosenberg. Comments are appreciated.

Cheers, Ben

Signed-off-by: Ben Konrath <bkonrath@redhat.com>

---
 org.spearce.egit-feature/.project         |   17 +++++++++
 org.spearce.egit-feature/build.properties |    1 +
 org.spearce.egit-feature/feature.xml      |   51 +++++++++++++++++++++++++++++
 org.spearce.egit/.classpath               |    6 +++
 org.spearce.egit/.project                 |   28 ++++++++++++++++
 org.spearce.egit/META-INF/MANIFEST.MF     |    7 ++++
 org.spearce.egit/about.ini                |   28 ++++++++++++++++
 org.spearce.egit/build.properties         |    3 ++
 org.spearce.egit/egit.png                 |  Bin 0 -> 226 bytes
 9 files changed, 141 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.egit-feature/.project
 create mode 100644 org.spearce.egit-feature/build.properties
 create mode 100644 org.spearce.egit-feature/feature.xml
 create mode 100644 org.spearce.egit/.classpath
 create mode 100644 org.spearce.egit/.project
 create mode 100644 org.spearce.egit/META-INF/MANIFEST.MF
 create mode 100644 org.spearce.egit/about.ini
 create mode 100644 org.spearce.egit/build.properties
 create mode 100644 org.spearce.egit/egit.png

diff --git a/org.spearce.egit-feature/.project b/org.spearce.egit-feature/.project
new file mode 100644
index 0000000..dac8f65
--- /dev/null
+++ b/org.spearce.egit-feature/.project
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>org.spearce.egit-feature</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.pde.FeatureBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.pde.FeatureNature</nature>
+	</natures>
+</projectDescription>
diff --git a/org.spearce.egit-feature/build.properties b/org.spearce.egit-feature/build.properties
new file mode 100644
index 0000000..64f93a9
--- /dev/null
+++ b/org.spearce.egit-feature/build.properties
@@ -0,0 +1 @@
+bin.includes = feature.xml
diff --git a/org.spearce.egit-feature/feature.xml b/org.spearce.egit-feature/feature.xml
new file mode 100644
index 0000000..8c98143
--- /dev/null
+++ b/org.spearce.egit-feature/feature.xml
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<feature
+      id="org.spearce.egit"
+      label="Eclipse Git Feature"
+      version="0.2.2"
+      provider-name="spearce.org"
+      plugin="org.spearce.egit">
+
+   <requires>
+      <import plugin="org.eclipse.core.runtime"/>
+      <import plugin="org.eclipse.team.core"/>
+      <import plugin="org.eclipse.core.resources"/>
+      <import plugin="org.eclipse.core.filesystem"/>
+      <import plugin="org.eclipse.ui"/>
+      <import plugin="org.eclipse.team.ui"/>
+      <import plugin="org.eclipse.jface.text"/>
+      <import plugin="org.eclipse.ui.ide"/>
+      <import plugin="org.eclipse.ui.workbench.texteditor"/>
+      <import plugin="org.eclipse.compare"/>
+      <import plugin="org.junit"/>
+   </requires>
+
+   <plugin
+         id="org.spearce.egit.core"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"
+         unpack="false"/>
+
+   <plugin
+         id="org.spearce.egit.ui"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"
+         unpack="false"/>
+
+   <plugin
+         id="org.spearce.egit"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"
+         unpack="false"/>
+
+   <plugin
+         id="org.spearce.jgit"
+         download-size="0"
+         install-size="0"
+         version="0.0.0"
+         unpack="false"/>
+
+</feature>
diff --git a/org.spearce.egit/.classpath b/org.spearce.egit/.classpath
new file mode 100644
index 0000000..acad1c2
--- /dev/null
+++ b/org.spearce.egit/.classpath
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
+	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>
diff --git a/org.spearce.egit/.project b/org.spearce.egit/.project
new file mode 100644
index 0000000..bbdc4e3
--- /dev/null
+++ b/org.spearce.egit/.project
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>org.spearce.egit</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.jdt.core.javabuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.ManifestBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.SchemaBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.pde.PluginNature</nature>
+		<nature>org.eclipse.jdt.core.javanature</nature>
+	</natures>
+</projectDescription>
diff --git a/org.spearce.egit/META-INF/MANIFEST.MF b/org.spearce.egit/META-INF/MANIFEST.MF
new file mode 100644
index 0000000..40da4e0
--- /dev/null
+++ b/org.spearce.egit/META-INF/MANIFEST.MF
@@ -0,0 +1,7 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: Java Git / Eclipse Git
+Bundle-SymbolicName: org.spearce.egit
+Bundle-Version: 0.2.2
+Bundle-Vendor: spearce.org
+Bundle-RequiredExecutionEnvironment: J2SE-1.5
diff --git a/org.spearce.egit/about.ini b/org.spearce.egit/about.ini
new file mode 100644
index 0000000..dcab31a
--- /dev/null
+++ b/org.spearce.egit/about.ini
@@ -0,0 +1,28 @@
+# about.ini
+# contains information about a feature
+# java.io.Properties file (ISO 8859-1 with "\" escapes)
+# "%key" are externalized strings defined in about.properties
+# This file does not need to be translated.
+
+# Property "aboutText" contains blurb for "About" dialog (translated)
+aboutText=Java Git / Eclipse Git (by Shawn Pearce) is a Java Git library and plugin for the Eclipse IDE.
+
+# Property "windowImage" contains path to window icon (16x16)
+# needed for primary features only
+
+# Property "featureImage" contains path to feature image (32x32)
+featureImage=egit.png
+
+# Property "aboutImage" contains path to product image (500x330 or 115x164)
+# needed for primary features only
+
+# Property "appName" contains name of the application (not translated)
+# needed for primary features only
+
+# Property "welcomePerspective" contains the id of the perspective in which the
+# welcome page is to be opened.
+# optional
+
+# Property "tipsAndTricksHref" contains the Help topic href to a tips and tricks page 
+# optional
+# tipsAndTricksHref=/org.eclipse.jdt.doc.user/tips/jdt_tips.html
\ No newline at end of file
diff --git a/org.spearce.egit/build.properties b/org.spearce.egit/build.properties
new file mode 100644
index 0000000..22d4e45
--- /dev/null
+++ b/org.spearce.egit/build.properties
@@ -0,0 +1,3 @@
+bin.includes = META-INF/,\
+               egit.png,\
+               about.ini
diff --git a/org.spearce.egit/egit.png b/org.spearce.egit/egit.png
new file mode 100644
index 0000000000000000000000000000000000000000..6782d4ab9b757aaa2c3d5edad42215ea02456836
GIT binary patch
literal 226
zc%17D@N?(olHy`uVBq!ia0vp^3LwnE1SJ1Ryj={WI14-?iy0XB6G519h5hnOpdfpR
zr>`sfbxuJ+YqhJ4<#s?J*%H@?66gHf+|;}h2Ir#G#FEq$h4Rdj426)4R0VfW-+=ci
zs(C<#5uPrNAs)xyPT9%JpuoeNd?c=?O``XtkE8f5g?WeG?ReJX!Qg2<Go@X*kws`m
zYWcec<yM{#)#eTV7R_Mb&A9PgU#?R3&?$y`+hv=lzZQBBWx}a4;eqf$hBiiBN4x4B
Rwm>@=JYD@<);T3K0RS$@N4)?5

literal 0
Hc$@<O00001

-- 
1.5.2.4

^ permalink raw reply related

* [PATCH 1/1] git-send-email: Add a --suppress-all option
From: Felipe Balbi @ 2007-09-17 18:33 UTC (permalink / raw)
  To: git; +Cc: Felipe Balbi

From: Felipe Balbi <felipe.lima@indt.org.br>

This patch adds a --suppress-all option to avoid sending emails
to everybody but the ones listed by --to option.

Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
---
 git-send-email.perl |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d8319d4..e845c5f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,6 +88,9 @@ Options:
    --suppress-from Suppress sending emails to yourself if your address
                   appears in a From: line. Defaults to off.
 
+   --suppress-all Suppress sending emails to everybody but addresses
+		  listed by --to option.
+
    --thread       Specify that the "In-Reply-To:" header should be set on all
                   emails. Defaults to on.
 
@@ -171,7 +174,7 @@ if ($@) {
 my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
+my ($thread, $chain_reply_to, $suppress_from, $suppress_all, $signed_off_cc, $cc_cmd);
 my ($smtp_server, $smtp_authuser, $smtp_authpass, $smtp_ssl);
 my ($identity, $aliasfiletype, @alias_files);
 
@@ -179,6 +182,7 @@ my %config_bool_settings = (
     "thread" => [\$thread, 1],
     "chainreplyto" => [\$chain_reply_to, 1],
     "suppressfrom" => [\$suppress_from, 0],
+    "suppressall" => [\$suppress_all, 0],
     "signedoffcc" => [\$signed_off_cc, 1],
     "smtpssl" => [\$smtp_ssl, 0],
 );
@@ -212,6 +216,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
+		    "suppress-all!" => \$suppress_all,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
@@ -662,8 +667,11 @@ foreach my $t (@files) {
 					$subject = $1;
 
 				} elsif (/^(Cc|From):\s+(.*)$/) {
-					if (unquote_rfc2047($2) eq $sender) {
-						next if ($suppress_from);
+					if (unquote_rfc2047($2)) {
+						next if ($suppress_all);
+					}
+					elsif (unquote_rfc2047($2) eq $sender) {
+						next if ($suppress_from|$suppress_all);
 					}
 					elsif ($1 eq 'From') {
 						$author = unquote_rfc2047($2);
@@ -700,7 +708,7 @@ foreach my $t (@files) {
 			}
 		} else {
 			$message .=  $_;
-			if (/^(Signed-off-by|Cc): (.*)$/i && $signed_off_cc) {
+			if (/^(Signed-off-by|Cc): (.*)$/i && $signed_off_cc && !$suppress_all) {
 				my $c = $2;
 				chomp $c;
 				push @cc, $c;
-- 
1.5.3.1.91.gd3392

^ permalink raw reply related

* Re: xmemdup patches
From: Pierre Habouzit @ 2007-09-17 17:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git ML
In-Reply-To: <Pine.LNX.4.64.0709171739010.28586@racer.site>

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

On Mon, Sep 17, 2007 at 04:39:23PM +0000, Johannes Schindelin wrote:
> On Mon, 17 Sep 2007, Pierre Habouzit wrote:
> >   I'm not a huge fan of "xmemdup" as I would not have supposed that a
> > function called like that would add the extra NUL, so I'm 100% okay with
> > someone coming up with any better name.
>
> xmemdupz()?

  Why not, it's definitely better than xmemdupasstring ;)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: metastore
From: Daniel Barkalow @ 2007-09-17 17:42 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: david, Johannes Schindelin, martin f krafft, git,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <7v7imp539u.fsf@gitster.siamese.dyndns.org>

On Sun, 16 Sep 2007, Junio C Hamano wrote:

> david@lang.hm writes:
> 
> >> Post-checkout trigger is something I can say I can live with
> >> without looking at the actual patch, but that does not mean it
> >> would be a better approach at all.
> >
> > we agree on this much at least :-)
> >
> >> I would not be able to answer the first question right now; that
> >> needs a patch to prove that it can be done with a well contained
> >> set of changes that results in a maintainable code.
> >
> > you cannot answer the question in the affirmitive, but you could say
> > that any changes in that area would be completely unacceptable to you
> > (and for a while it sounded like you were saying exactly that). in
> > which case any effort put into preparing patches would be a waste of
> > time
> 
> I tend to disagree.  It's far from a waste of time.  While, as I
> said, I am skeptical that such a patch would be small impact, if
> it helps people's needs, somebody will pick it up and carry
> forward, even if that somebody is not me.  It can then mature
> out of tree and later could be merged.  We simply do not know
> unless somebody tries.  And I am quite happy that you seem to be
> motivated enough to see how it goes.

There's certainly the possibility that a changeset could consist of some 
patches that make the index/filesystem handling more clear, some patches 
that make the tree/index handling more clear, and some patches that allow 
a hook to replace one of these entirely. Things can be a lot more 
acceptable if the intrusive changes are improvements for the 
maintainability of the normal case, and the special case code is no longer 
intrusive at all.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: metastore
From: david @ 2007-09-17 17:17 UTC (permalink / raw)
  To: martin f krafft
  Cc: git, Daniel Barkalow, Junio C Hamano, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <20070917133000.GD16773@lapse.madduck.net>

On Mon, 17 Sep 2007, martin f krafft wrote:

> also sprach david@lang.hm <david@lang.hm> [2007.09.17.0037 +0200]:
>>> While we're at it, you probably don't even want to write the
>>> permission file to the live filesystem. It's just one more thing
>>> that could leak information, and changes to the permissions of
>>> files that you record by committing the live filesystem would
>>> presumably be done by changing the permissions of files in the
>>> filesystem, not by changing the text file.
>>
>> the permissions and ACL's can be queried directly from the
>> filesystem, so I don't see any security problems with writing the
>> permission file to the filesystem.
>>
>> changing the permissions would be done by changing the files
>> themselves (when you are running as root on a filesystem that
>> supports the changes, otherwise it would need to fall back to
>> writing the file and getting the changes there, but that should be
>> able to be a local config option)
>>
>> I don't like the idea of having a file that doesn't appear on the
>> local filesystem at any point, it just makes troubleshooting too
>> hard.
>
> Reading over your thoughts, I get this uneasy feeling about such
> a permissions file, because it stores redundant information, and
> redundant information has a tendency to get out of sync. If we
> cannot attach attributes to objects in the git database, then
> I understand the need for such a metastore. But I don't think it
> should be checked out and visible, or maybe we should think of it
> not in terms of a file anyway, but a metastore. Or how do you want
> to resolve the situation when a user might edit the file, changing
> a mode from 644 to 640, while in the filesystem, it was changed by
> other means to 600.

each local repository would need to be configured to either recreate the 
permissions file at checkin time or to use the permission file and ignore 
the actual permissions on the file.

while I agree that it would be ideal to store this data inside git, I'm 
more interested in getting a functional implementation, and given the 
reluctance of the git core team to allow any changes to support this 
use-case anything that can be done to minimize the changes needed to 
support this use-case is a good thing.

> .gitattributes is a different story since it stores git-specificy
> attributes, which are present nowhere else in the checkout.
>
> I still maintain it would be best if git allowed extra data to be
> attached to object nodes. When you start thinking about
> cherry-picking or even simple merges, I think that makes most sense.
> And we don't need conflict markers, we could employ an iterative
> merge process as e.g. git-rebase uses:
>
>  "a conflict has been found in the file mode of ...
>   ... 2750 vs. 2755 ...
>   please set the file mode as it should be and do git-merge
>   --continue. Or git-merge --abort. ..."

and there's nothing to prevent the checkin hook from running such a 
comparison if you want it to.

>>> (Of course, you could check out the same commits as ordinary source, with
>>> developer-owned 644 files and a 644 "permissions" file, and there you'd
>>> have the permissions file appear in the work tree, and you could edit it
>>> and check it in in a totally mundane way.)
>>
>> right, and the same thing if the filesystem doesn't support something in the
>> permission file.
>
> I'd much rather see something like `git-attr chmod 644
> file-in-index` to make this change, rather than a file, which
> introduces the potential for syntax errors.

first make this useable, then if it starts getting used widely (which 
would not at all surprise me, many distros are looking for good options 
for doing this sort of thing, I wouldn't be surprised to see several of 
them start useing git if it did the job well) things can be moved from 
external scripts and storage to internal capabilities as appropriate.

David Lang

^ permalink raw reply

* Re: metastore
From: martin f krafft @ 2007-09-17 13:30 UTC (permalink / raw)
  To: git
  Cc: david, Daniel Barkalow, Junio C Hamano, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161507130.24221@asgard.lang.hm>

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

also sprach david@lang.hm <david@lang.hm> [2007.09.17.0037 +0200]:
>> While we're at it, you probably don't even want to write the
>> permission file to the live filesystem. It's just one more thing
>> that could leak information, and changes to the permissions of
>> files that you record by committing the live filesystem would
>> presumably be done by changing the permissions of files in the
>> filesystem, not by changing the text file.
>
> the permissions and ACL's can be queried directly from the
> filesystem, so I don't see any security problems with writing the
> permission file to the filesystem.
>
> changing the permissions would be done by changing the files
> themselves (when you are running as root on a filesystem that
> supports the changes, otherwise it would need to fall back to
> writing the file and getting the changes there, but that should be
> able to be a local config option)
>
> I don't like the idea of having a file that doesn't appear on the
> local filesystem at any point, it just makes troubleshooting too
> hard.

Reading over your thoughts, I get this uneasy feeling about such
a permissions file, because it stores redundant information, and
redundant information has a tendency to get out of sync. If we
cannot attach attributes to objects in the git database, then
I understand the need for such a metastore. But I don't think it
should be checked out and visible, or maybe we should think of it
not in terms of a file anyway, but a metastore. Or how do you want
to resolve the situation when a user might edit the file, changing
a mode from 644 to 640, while in the filesystem, it was changed by
other means to 600.

.gitattributes is a different story since it stores git-specificy
attributes, which are present nowhere else in the checkout.

I still maintain it would be best if git allowed extra data to be
attached to object nodes. When you start thinking about
cherry-picking or even simple merges, I think that makes most sense.
And we don't need conflict markers, we could employ an iterative
merge process as e.g. git-rebase uses:

  "a conflict has been found in the file mode of ...
   ... 2750 vs. 2755 ...
   please set the file mode as it should be and do git-merge
   --continue. Or git-merge --abort. ..."

>> (Of course, you could check out the same commits as ordinary source, with
>> developer-owned 644 files and a 644 "permissions" file, and there you'd
>> have the permissions file appear in the work tree, and you could edit it
>> and check it in in a totally mundane way.)
>
> right, and the same thing if the filesystem doesn't support something in the 
> permission file.

I'd much rather see something like `git-attr chmod 644
file-in-index` to make this change, rather than a file, which
introduces the potential for syntax errors.

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
"to me, vi is zen. to use vi is to practice zen. every command is
 a koan. profound to the user, unintelligible to the uninitiated.
 you discover truth everytime you use it."
                                       -- reddy ät lion.austin.ibm.com
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-09-17 13:31 UTC (permalink / raw)
  To: git
  Cc: Jan Hudec, Johannes Schindelin, Daniel Barkalow,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <20070916155147.GA30476@efreet.light.src>

[-- Attachment #1: Type: text/plain, Size: 2121 bytes --]

also sprach Jan Hudec <bulb@ucw.cz> [2007.09.16.1751 +0200]:
> > If the filesystem does not support owners, chown() would not
> > exist. I actually tend to think of things the other way around:
> > instead of a fallback when chown() does not work (what would
> > such a fallback be other than not chown()ing?), it would only
> > try chown() if such functionality existed.
>·
> There's a problem. You need to know that the functionality is
> missing and not try to read attributes back, but instead consider
> them unchanged. Nothing that can't be taken care of, but it needs
> to be handled carefuly.

This is a good consideration. One way of implementing this seems to
be to iterate over all file attributes recorded in the object cache
(or metastore) and try to apply each. For every attribute that was
properly applied to the worktree, a note is attached to the object's
data in the index. Tools identifying differences between index and
worktree would then only pay attention to these attributes.

> But if you tar that up again, the owners will be different. But
> you don't want the change.

As per my above suggestion, this would solve itself. Untarring as
non-root simply means that the chmod/chown/whatever calls would fail
or not be tried at all. Thus, they would not be recorded in the
index and later commits would never consider changes to these
attributes.

One could probably simplify the implementation such that failure to
chmod/chown/whatever a single file would make the attribute be 
ignored when worktree and index are compared. Then, it would all 
boil down to a combination of configuration and functionality: the
attributes the user wants to have tracked (configuration) and those
which can be applied to the worktree when logically and'ed result in
the final mask of attributes to consider when identifying changes.

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
a gourmet concerned about calories
is like a punter eyeing the clock.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: xmemdup patches
From: Johannes Schindelin @ 2007-09-17 16:39 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Git ML
In-Reply-To: <20070917161113.GB460@artemis.corp>

Hi,

On Mon, 17 Sep 2007, Pierre Habouzit wrote:

>   I'm not a huge fan of "xmemdup" as I would not have supposed that a
> function called like that would add the extra NUL, so I'm 100% okay with
> someone coming up with any better name.

xmemdupz()?

Ciao,
Dscho

^ permalink raw reply

* [PATCH] git-merge: add option --no-ff
From: Lars Hjemli @ 2007-09-17 16:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Andreas Ericsson, Johannes Schindelin, Eric Wong, Chris Shoemaker,
	git
In-Reply-To: <8c5c35580709170817s467fa7dv375952f872bba0e3@mail.gmail.com>

This option forces fast-forward merges to create a "true" merge commit,
i.e. a commit with multiple parents.

Although a fast-forward merge would normally be the right thing to do with
git branches, it is suboptimal when operating on git-svn branches since it
makes 'git-svn dcommit' fail to recognize the correct upstream subversion
branch. But performing such a merge with --no-ff specified will both make
git-svn dcommit recognize the correct upstream and create the logically
correct history in subversion (the merge performed in git will be recorded
as a single revision in subversion, not as a series of revisions seemingly
cherry-picked from the merged branch).

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

When updating git-svn.txt, I noticed that we might want to update the 
section "DESIGN PHILOSOPHY". Eric?


 Documentation/git-svn.txt       |   13 +++++++++++++
 Documentation/merge-options.txt |    5 +++++
 git-merge.sh                    |   13 +++++++++++--
 t/t6028-merge-up-to-date.sh     |   25 +++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index be2e34e..c510c21 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -475,6 +475,19 @@ use 'git-svn rebase' to update your work branch instead of 'git pull' or
 when committing into SVN, which can lead to merge commits reversing
 previous commits in SVN.
 
+If you use 'git-svn dcommit' to commit your local work to the upstream
+subversion branch, merge commits are usually handled correctly, i.e.
+git-svn will only follow the first parent of each merge commit and create
+a single subversion revision for each of them. An exception is when two
+subversion branches has been merged locally and the merge ended up as a
+fast-forward operation. This will make git-svn belive that there are no
+local changes to dcommit. To work around this issue, one can redo the
+merge using the --no-ff option:
+
+       $ git reset --hard HEAD@{1}   ## undo the fast-forward merge
+       $ git merge --no-ff <branch>
+
+
 DESIGN PHILOSOPHY
 -----------------
 Merge tracking in Subversion is lacking and doing branched development
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index d64c259..b34b888 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -25,3 +25,8 @@
 	If there is no `-s` option, a built-in list of strategies
 	is used instead (`git-merge-recursive` when merging a single
 	head, `git-merge-octopus` otherwise).
+
+--no-ff::
+	Force the creation of a merge commit even when the merge would
+	have resolved as a fast-forward operation. See gitlink:git-svn[1]
+	for a use-case for this option.
diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..13b98e6 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
-USAGE='[-n] [--summary] [--no-commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
+USAGE='[-n] [--summary] [--no-commit] [--no-ff] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
 
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -165,6 +165,10 @@ do
 		merge_msg="$1"
 		have_message=t
 		;;
+	--no-ff)
+		no_ff=t
+		no_fast_forward_strategies=$all_strategies
+		;;
 	-*)	usage ;;
 	*)	break ;;
 	esac
@@ -444,7 +448,12 @@ done
 # auto resolved the merge cleanly.
 if test '' != "$result_tree"
 then
-    parents=$(git show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
+    if test $no_ff = 't'
+    then
+        parents=$(git rev-parse "$head" "$@" | sed -e 's/^/-p /')
+    else
+        parents=$(git show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
+    fi
     result_commit=$(printf '%s\n' "$merge_msg" | git commit-tree $result_tree $parents) || exit
     finish "$result_commit" "Merge made by $wt_strategy."
     dropsave
diff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh
index f8f3e3f..afd74e2 100755
--- a/t/t6028-merge-up-to-date.sh
+++ b/t/t6028-merge-up-to-date.sh
@@ -10,12 +10,14 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m initial &&
 	git tag c0 &&
+	c0=$(git rev-parse c0)
 
 	echo second >file &&
 	git add file &&
 	test_tick &&
 	git commit -m second &&
 	git tag c1 &&
+	c1=$(git rev-parse c1)
 	git branch test
 '
 
@@ -41,6 +43,16 @@ test_expect_success 'merge -s recursive fast-forward' '
 
 '
 
+test_expect_success 'merge -s recursive --no-ff' '
+
+	git reset --hard c0 &&
+	test_tick &&
+	git merge -s recursive --no-ff c1 &&
+	test $c0 = $(git rev-parse HEAD^1) &&
+	test $c1 = $(git rev-parse HEAD^2)
+
+'
+
 test_expect_success 'merge -s ours up-to-date' '
 
 	git reset --hard c1 &&
@@ -63,6 +75,19 @@ test_expect_success 'merge -s ours fast-forward' '
 
 '
 
+test_expect_success 'merge -s ours --no-ff' '
+
+	git reset --hard c0 &&
+	test_tick &&
+	git merge -s ours --no-ff c1 &&
+	expect=$(git rev-parse c0^{tree}) &&
+	current=$(git rev-parse HEAD^{tree}) &&
+	test "$expect" = "$current" &&
+	test $c0 = $(git rev-parse HEAD^1) &&
+	test $c1 = $(git rev-parse HEAD^2)
+
+'
+
 test_expect_success 'merge -s subtree up-to-date' '
 
 	git reset --hard c1 &&
-- 
1.5.3.1.92.g2f5e

^ permalink raw reply related

* Re: [PATCH] git-merge: add option --no-ff
From: Lars Hjemli @ 2007-09-17 16:14 UTC (permalink / raw)
  To: Chris Shoemaker
  Cc: Johannes Schindelin, Andreas Ericsson, Junio C Hamano, git,
	Eric Wong
In-Reply-To: <20070917160755.GA11287@pe.Belkin>

On 9/17/07, Chris Shoemaker <c.shoemaker@cox.net> wrote:
> On Mon, Sep 17, 2007 at 04:12:56PM +0200, Lars Hjemli wrote:
> > On 9/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > But then, I do not use svn branches here, and that might be the problem?
> >
> > Probably. The case I'm trying to solve is:
> >   -git-svn branch A is merged into git-svn branch B
> >   -A is a fast-forward of B
>
> Ah, now I see what you mean.  But, IIUC, if you want to dcommit your
> merge, you should treat it the way svn treats it, with git-merge
> --squash.  Then, dcommit won't be confused about the branch you're
> committing to.

Yeah, --squash is a viable option (I _almost_ used it ;-) but I wanted
to keep the merge-history on the git side (without modifying the
grafts-file).

-- 
larsh

^ permalink raw reply

* Re: xmemdup patches
From: Pierre Habouzit @ 2007-09-17 16:14 UTC (permalink / raw)
  To: Git ML
In-Reply-To: <20070917161113.GB460@artemis.corp>

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On lun, sep 17, 2007 at 04:11:13 +0000, Pierre Habouzit wrote:
>   There were 41 of those places.

  I forgot to say, I replaced _some_ xstrndup uses with a xmemdup when
it was clear to me that xmemdup was Okay. I left some alone when it
wasn't obvious in the first 10 seconds that xmemdup would work the same
way.

  The real goal is code brevity and readability, xstrndup or xmemdup are
the same in that regard, and I don't think the small performance gain is
really worth the headache.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 2/2] Use xmemdup in many places.
From: Pierre Habouzit @ 2007-09-15 22:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20070917161113.GB460@artemis.corp>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 attr.c                  |    7 +------
 builtin-add.c           |    8 ++------
 builtin-apply.c         |   11 ++---------
 builtin-fetch--tool.c   |    6 +-----
 builtin-fmt-merge-msg.c |   17 ++++++-----------
 builtin-for-each-ref.c  |   40 +++++++++-------------------------------
 builtin-log.c           |   12 ++----------
 builtin-ls-files.c      |    9 +--------
 builtin-mv.c            |    5 +----
 builtin-revert.c        |    4 +---
 builtin-shortlog.c      |   11 ++---------
 commit.c                |   16 ++++++----------
 connect.c               |    4 +---
 convert.c               |    7 +------
 diff.c                  |   13 ++-----------
 diffcore-order.c        |    7 ++-----
 fast-import.c           |    4 +---
 http-push.c             |    9 ++-------
 imap-send.c             |   20 +++++---------------
 merge-recursive.c       |   19 ++++---------------
 refs.c                  |   12 ++++--------
 sha1_file.c             |   12 +++---------
 tag.c                   |    4 +---
 23 files changed, 60 insertions(+), 197 deletions(-)

diff --git a/attr.c b/attr.c
index 1293993..fbfc2eb 100644
--- a/attr.c
+++ b/attr.c
@@ -160,12 +160,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
 		else if (!equals)
 			e->setto = ATTR__TRUE;
 		else {
-			char *value;
-			int vallen = ep - equals;
-			value = xmalloc(vallen);
-			memcpy(value, equals+1, vallen-1);
-			value[vallen-1] = 0;
-			e->setto = value;
+			e->setto = xmemdup(equals + 1, ep - equals - 1);
 		}
 		e->attr = git_attr(cp, len);
 	}
diff --git a/builtin-add.c b/builtin-add.c
index 0d7d0ce..55eedd8 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -71,12 +71,8 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 	baselen = common_prefix(pathspec);
 	path = ".";
 	base = "";
-	if (baselen) {
-		char *common = xmalloc(baselen + 1);
-		memcpy(common, *pathspec, baselen);
-		common[baselen] = 0;
-		path = base = common;
-	}
+	if (baselen)
+		path = base = xmemdup(*pathspec, baselen);
 
 	/* Read the directory and prune it */
 	read_directory(dir, path, base, baselen, pathspec);
diff --git a/builtin-apply.c b/builtin-apply.c
index 05011bb..900d0a7 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -293,11 +293,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)
 			return def;
 	}
 
-	name = xmalloc(len + 1);
-	memcpy(name, start, len);
-	name[len] = 0;
-	free(def);
-	return name;
+	return xmemdup(start, len);
 }
 
 static int count_slashes(const char *cp)
@@ -687,10 +683,7 @@ static char *git_header_name(char *line, int llen)
 					break;
 			}
 			if (second[len] == '\n' && !memcmp(name, second, len)) {
-				char *ret = xmalloc(len + 1);
-				memcpy(ret, name, len);
-				ret[len] = 0;
-				return ret;
+				return xmemdup(name, len);
 			}
 		}
 	}
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 514a3cc..327f555 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -222,19 +222,15 @@ static char *find_local_name(const char *remote_name, const char *refs,
 		}
 		if (!strncmp(remote_name, ref, len) && ref[len] == ':') {
 			const char *local_part = ref + len + 1;
-			char *ret;
 			int retlen;
 
 			if (!next)
 				retlen = strlen(local_part);
 			else
 				retlen = next - local_part;
-			ret = xmalloc(retlen + 1);
-			memcpy(ret, local_part, retlen);
-			ret[retlen] = 0;
 			*force_p = single_force;
 			*not_for_merge_p = not_for_merge;
-			return ret;
+			return xmemdup(local_part, retlen);
 		}
 		ref = next;
 	}
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index ae60fcc..6dd217a 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -140,12 +140,10 @@ static int handle_line(char *line)
 	if (!strcmp(".", src) || !strcmp(src, origin)) {
 		int len = strlen(origin);
 		if (origin[0] == '\'' && origin[len - 1] == '\'') {
-			char *new_origin = xmalloc(len - 1);
-			memcpy(new_origin, origin + 1, len - 2);
-			new_origin[len - 2] = 0;
-			origin = new_origin;
-		} else
+			origin = xmemdup(origin + 1, len - 2);
+		} else {
 			origin = xstrdup(origin);
+		}
 	} else {
 		char *new_origin = xmalloc(strlen(origin) + strlen(src) + 5);
 		sprintf(new_origin, "%s of %s", origin, src);
@@ -211,14 +209,11 @@ static void shortlog(const char *name, unsigned char *sha1,
 
 		bol += 2;
 		eol = strchr(bol, '\n');
-
 		if (eol) {
-			int len = eol - bol;
-			oneline = xmalloc(len + 1);
-			memcpy(oneline, bol, len);
-			oneline[len] = 0;
-		} else
+			oneline = xmemdup(bol, eol - bol);
+		} else {
 			oneline = xstrdup(bol);
+		}
 		append_to_list(&subjects, oneline, NULL);
 	}
 
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 0afa1c5..287d52a 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -87,7 +87,6 @@ static int used_atom_cnt, sort_atom_limit, need_tagged;
 static int parse_atom(const char *atom, const char *ep)
 {
 	const char *sp;
-	char *n;
 	int i, at;
 
 	sp = atom;
@@ -120,10 +119,7 @@ static int parse_atom(const char *atom, const char *ep)
 			     (sizeof *used_atom) * used_atom_cnt);
 	used_atom_type = xrealloc(used_atom_type,
 				  (sizeof(*used_atom_type) * used_atom_cnt));
-	n = xmalloc(ep - atom + 1);
-	memcpy(n, atom, ep - atom);
-	n[ep-atom] = 0;
-	used_atom[at] = n;
+	used_atom[at] = xmemdup(atom, ep - atom);
 	used_atom_type[at] = valid_atom[i].cmp_type;
 	return at;
 }
@@ -305,46 +301,28 @@ static const char *find_wholine(const char *who, int wholen, const char *buf, un
 static const char *copy_line(const char *buf)
 {
 	const char *eol = strchr(buf, '\n');
-	char *line;
-	int len;
 	if (!eol)
 		return "";
-	len = eol - buf;
-	line = xmalloc(len + 1);
-	memcpy(line, buf, len);
-	line[len] = 0;
-	return line;
+	return xmemdup(buf, eol - buf);
 }
 
 static const char *copy_name(const char *buf)
 {
-	const char *eol = strchr(buf, '\n');
-	const char *eoname = strstr(buf, " <");
-	char *line;
-	int len;
-	if (!(eoname && eol && eoname < eol))
-		return "";
-	len = eoname - buf;
-	line = xmalloc(len + 1);
-	memcpy(line, buf, len);
-	line[len] = 0;
-	return line;
+	const char *cp;
+	for (cp = buf; *cp != '\n'; cp++) {
+		if (!strncmp(cp, " <", 2))
+			return xmemdup(buf, cp - buf);
+	}
+	return "";
 }
 
 static const char *copy_email(const char *buf)
 {
 	const char *email = strchr(buf, '<');
 	const char *eoemail = strchr(email, '>');
-	char *line;
-	int len;
 	if (!email || !eoemail)
 		return "";
-	eoemail++;
-	len = eoemail - email;
-	line = xmalloc(len + 1);
-	memcpy(line, email, len);
-	line[len] = 0;
-	return line;
+	return xmemdup(email, eoemail + 1 - email);
 }
 
 static void grab_date(const char *buf, struct atom_value *v)
diff --git a/builtin-log.c b/builtin-log.c
index 60819d1..f61a568 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -441,8 +441,6 @@ static const char *clean_message_id(const char *msg_id)
 {
 	char ch;
 	const char *a, *z, *m;
-	char *n;
-	size_t len;
 
 	m = msg_id;
 	while ((ch = *m) && (isspace(ch) || (ch == '<')))
@@ -458,11 +456,7 @@ static const char *clean_message_id(const char *msg_id)
 		die("insane in-reply-to: %s", msg_id);
 	if (++z == m)
 		return a;
-	len = z - a;
-	n = xmalloc(len + 1);
-	memcpy(n, a, len);
-	n[len] = 0;
-	return n;
+	return xmemdup(a, z - a);
 }
 
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
@@ -541,9 +535,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			endpos = strchr(committer, '>');
 			if (!endpos)
 				die("bogos committer info %s\n", committer);
-			add_signoff = xmalloc(endpos - committer + 2);
-			memcpy(add_signoff, committer, endpos - committer + 1);
-			add_signoff[endpos - committer + 1] = 0;
+			add_signoff = xmemdup(committer, endpos - committer + 1);
 		}
 		else if (!strcmp(argv[i], "--attach")) {
 			rev.mime_boundary = git_version_string;
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 6c1db86..64f0011 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -299,7 +299,6 @@ static void prune_cache(const char *prefix)
 static const char *verify_pathspec(const char *prefix)
 {
 	const char **p, *n, *prev;
-	char *real_prefix;
 	unsigned long max;
 
 	prev = NULL;
@@ -326,14 +325,8 @@ static const char *verify_pathspec(const char *prefix)
 	if (prefix_offset > max || memcmp(prev, prefix, prefix_offset))
 		die("git-ls-files: cannot generate relative filenames containing '..'");
 
-	real_prefix = NULL;
 	prefix_len = max;
-	if (max) {
-		real_prefix = xmalloc(max + 1);
-		memcpy(real_prefix, prev, max);
-		real_prefix[max] = 0;
-	}
-	return real_prefix;
+	return max ? xmemdup(prev, max) : NULL;
 }
 
 /*
diff --git a/builtin-mv.c b/builtin-mv.c
index b95b7d2..9dacc95 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -22,10 +22,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
 	for (i = 0; i < count; i++) {
 		int length = strlen(result[i]);
 		if (length > 0 && result[i][length - 1] == '/') {
-			char *without_slash = xmalloc(length);
-			memcpy(without_slash, result[i], length - 1);
-			without_slash[length - 1] = '\0';
-			result[i] = without_slash;
+			result[i] = xmemdup(result[i], length - 1);
 		}
 		if (base_name) {
 			const char *last_slash = strrchr(result[i], '/');
diff --git a/builtin-revert.c b/builtin-revert.c
index 499bbe7..4d96d3f 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -168,9 +168,7 @@ static void set_author_ident_env(const char *message)
 			char *line, *pend, *email, *timestamp;
 
 			p += 7;
-			line = xmalloc(eol + 1 - p);
-			memcpy(line, p, eol - p);
-			line[eol - p] = '\0';
+			line = xmemdup(p, eol - p);
 			email = strchr(line, '<');
 			if (!email)
 				die ("Could not extract author email from %s",
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 16af619..a6cbe39 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -39,10 +39,7 @@ static void insert_author_oneline(struct path_list *list,
 	while (authorlen > 0 && isspace(author[authorlen - 1]))
 		authorlen--;
 
-	buffer = xmalloc(authorlen + 1);
-	memcpy(buffer, author, authorlen);
-	buffer[authorlen] = '\0';
-
+	buffer = xmemdup(author, authorlen);
 	item = path_list_insert(buffer, list);
 	if (item->util == NULL)
 		item->util = xcalloc(1, sizeof(struct path_list));
@@ -66,13 +63,9 @@ static void insert_author_oneline(struct path_list *list,
 		oneline++;
 		onelinelen--;
 	}
-
 	while (onelinelen > 0 && isspace(oneline[onelinelen - 1]))
 		onelinelen--;
-
-	buffer = xmalloc(onelinelen + 1);
-	memcpy(buffer, oneline, onelinelen);
-	buffer[onelinelen] = '\0';
+	buffer = xmemdup(oneline, onelinelen);
 
 	if (dot3) {
 		int dot3len = strlen(dot3);
diff --git a/commit.c b/commit.c
index 85889f9..2778621 100644
--- a/commit.c
+++ b/commit.c
@@ -628,11 +628,7 @@ static char *get_header(const struct commit *commit, const char *key)
 		if (eol - line > key_len &&
 		    !strncmp(line, key, key_len) &&
 		    line[key_len] == ' ') {
-			int len = eol - line - key_len;
-			char *ret = xmalloc(len);
-			memcpy(ret, line + key_len + 1, len - 1);
-			ret[len - 1] = '\0';
-			return ret;
+			return xmemdup(line + key_len + 1, eol - line - key_len - 1);
 		}
 		line = next;
 	}
@@ -709,7 +705,7 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	start = end + 1;
 	while (end > 0 && isspace(msg[end - 1]))
 		end--;
-	table[0].value = xstrndup(msg, end);
+	table[0].value = xmemdup(msg, end);
 
 	if (start >= len)
 		return;
@@ -721,7 +717,7 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	if (end >= len)
 		return;
 
-	table[1].value = xstrndup(msg + start, end - start);
+	table[1].value = xmemdup(msg + start, end - start);
 
 	/* parse date */
 	for (start = end + 1; start < len && isspace(msg[start]); start++)
@@ -732,7 +728,7 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	if (msg + start == ep)
 		return;
 
-	table[5].value = xstrndup(msg + start, ep - (msg + start));
+	table[5].value = xmemdup(msg + start, ep - (msg + start));
 
 	/* parse tz */
 	for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
@@ -859,7 +855,7 @@ void format_commit_message(const struct commit *commit,
 			; /* do nothing */
 
 		if (state == SUBJECT) {
-			table[ISUBJECT].value = xstrndup(msg + i, eol - i);
+			table[ISUBJECT].value = xmemdup(msg + i, eol - i);
 			i = eol;
 		}
 		if (i == eol) {
@@ -875,7 +871,7 @@ void format_commit_message(const struct commit *commit,
 					msg + i + 10, eol - i - 10);
 		else if (!prefixcmp(msg + i, "encoding "))
 			table[IENCODING].value =
-				xstrndup(msg + i + 9, eol - i - 9);
+				xmemdup(msg + i + 9, eol - i - 9);
 		i = eol;
 	}
 	if (msg[i])
diff --git a/connect.c b/connect.c
index 8b1e993..779de85 100644
--- a/connect.c
+++ b/connect.c
@@ -393,9 +393,7 @@ static int git_proxy_command_options(const char *var, const char *value)
 			if (matchlen == 4 &&
 			    !memcmp(value, "none", 4))
 				matchlen = 0;
-			git_proxy_command = xmalloc(matchlen + 1);
-			memcpy(git_proxy_command, value, matchlen);
-			git_proxy_command[matchlen] = 0;
+			git_proxy_command = xmemdup(value, matchlen);
 		}
 		return 0;
 	}
diff --git a/convert.c b/convert.c
index 508d30b..3a0a9ed 100644
--- a/convert.c
+++ b/convert.c
@@ -323,13 +323,8 @@ static int read_convert_config(const char *var, const char *value)
 		if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
 			break;
 	if (!drv) {
-		char *namebuf;
 		drv = xcalloc(1, sizeof(struct convert_driver));
-		namebuf = xmalloc(namelen + 1);
-		memcpy(namebuf, name, namelen);
-		namebuf[namelen] = 0;
-		drv->name = namebuf;
-		drv->next = NULL;
+		drv->name = xmemdup(name, namelen);
 		*user_convert_tail = drv;
 		user_convert_tail = &(drv->next);
 	}
diff --git a/diff.c b/diff.c
index a5b69ed..f2e9a44 100644
--- a/diff.c
+++ b/diff.c
@@ -83,13 +83,8 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val
 		if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
 			break;
 	if (!drv) {
-		char *namebuf;
 		drv = xcalloc(1, sizeof(struct ll_diff_driver));
-		namebuf = xmalloc(namelen + 1);
-		memcpy(namebuf, name, namelen);
-		namebuf[namelen] = 0;
-		drv->name = namebuf;
-		drv->next = NULL;
+		drv->name = xmemdup(name, namelen);
 		if (!user_diff_tail)
 			user_diff_tail = &user_diff;
 		*user_diff_tail = drv;
@@ -126,12 +121,8 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v
 		if (!strncmp(pp->name, name, namelen) && !pp->name[namelen])
 			break;
 	if (!pp) {
-		char *namebuf;
 		pp = xcalloc(1, sizeof(*pp));
-		namebuf = xmalloc(namelen + 1);
-		memcpy(namebuf, name, namelen);
-		namebuf[namelen] = 0;
-		pp->name = namebuf;
+		pp->name = xmemdup(name, namelen);
 		pp->next = funcname_pattern_list;
 		funcname_pattern_list = pp;
 	}
diff --git a/diffcore-order.c b/diffcore-order.c
index 2a4bd82..e20ab98 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -48,11 +48,8 @@ static void prepare_order(const char *orderfile)
 				if (*ep == '\n') {
 					*ep = 0;
 					order[cnt] = cp;
-				}
-				else {
-					order[cnt] = xmalloc(ep-cp+1);
-					memcpy(order[cnt], cp, ep-cp);
-					order[cnt][ep-cp] = 0;
+				} else {
+					order[cnt] = xmemdup(cp, ep - cp);
 				}
 				cnt++;
 			}
diff --git a/fast-import.c b/fast-import.c
index e2a4834..d5e4b1d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1864,9 +1864,7 @@ static void file_change_cr(struct branch *b, int rename)
 		endp = strchr(s, ' ');
 		if (!endp)
 			die("Missing space after source: %s", command_buf.buf);
-		s_uq = xmalloc(endp - s + 1);
-		memcpy(s_uq, s, endp - s);
-		s_uq[endp - s] = 0;
+		s_uq = xmemdup(s, endp - s);
 	}
 	s = s_uq;
 
diff --git a/http-push.c b/http-push.c
index 7c3720f..f5e4266 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1271,10 +1271,7 @@ xml_cdata(void *userData, const XML_Char *s, int len)
 {
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
 	free(ctx->cdata);
-	ctx->cdata = xmalloc(len + 1);
-	/* NB: 's' is not null-terminated, can not use strlcpy here */
-	memcpy(ctx->cdata, s, len);
-	ctx->cdata[len] = '\0';
+	ctx->cdata = xmemdup(s, len);
 }
 
 static struct remote_lock *lock_remote(const char *path, long timeout)
@@ -2172,9 +2169,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
 	if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
-		*symref = xmalloc(buffer.posn - 5);
-		memcpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 6);
-		(*symref)[buffer.posn - 6] = '\0';
+		*symref = xmemdup(buffer.buffer + 5, buffer.posn - 6);
 	} else {
 		get_sha1_hex(buffer.buffer, sha1);
 	}
diff --git a/imap-send.c b/imap-send.c
index 86e4a0f..b07decc 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -623,9 +623,7 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 					goto bail;
 			cur->len = s - p;
 			s++;
-			cur->val = xmalloc( cur->len + 1 );
-			memcpy( cur->val, p, cur->len );
-			cur->val[cur->len] = 0;
+			cur->val = xmemdup(p, cur->len);
 		} else {
 			/* atom */
 			p = s;
@@ -633,12 +631,10 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 				if (level && *s == ')')
 					break;
 			cur->len = s - p;
-			if (cur->len == 3 && !memcmp ("NIL", p, 3))
+			if (cur->len == 3 && !memcmp ("NIL", p, 3)) {
 				cur->val = NIL;
-			else {
-				cur->val = xmalloc( cur->len + 1 );
-				memcpy( cur->val, p, cur->len );
-				cur->val[cur->len] = 0;
+			} else {
+				cur->val = xmemdup(p, cur->len);
 			}
 		}
 
@@ -1221,13 +1217,7 @@ split_msg( msg_data_t *all_msgs, msg_data_t *msg, int *ofs )
 	if (p)
 		msg->len = &p[1] - data;
 
-	msg->data = xmalloc( msg->len + 1 );
-	if (!msg->data)
-		return 0;
-
-	memcpy( msg->data, data, msg->len );
-	msg->data[ msg->len ] = 0;
-
+	msg->data = xmemdup(data, msg->len);
 	*ofs += msg->len;
 	return 1;
 }
diff --git a/merge-recursive.c b/merge-recursive.c
index 19d5f3b..e38011b 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -432,19 +432,15 @@ static int update_stages(const char *path, struct diff_filespec *o,
 
 static int remove_path(const char *name)
 {
-	int ret, len;
+	int ret;
 	char *slash, *dirs;
 
 	ret = unlink(name);
 	if (ret)
 		return ret;
-	len = strlen(name);
-	dirs = xmalloc(len+1);
-	memcpy(dirs, name, len);
-	dirs[len] = '\0';
+	dirs = xstrdup(name);
 	while ((slash = strrchr(name, '/'))) {
 		*slash = '\0';
-		len = slash - name;
 		if (rmdir(name) != 0)
 			break;
 	}
@@ -578,9 +574,7 @@ static void update_file_flags(const unsigned char *sha,
 			flush_buffer(fd, buf, size);
 			close(fd);
 		} else if (S_ISLNK(mode)) {
-			char *lnk = xmalloc(size + 1);
-			memcpy(lnk, buf, size);
-			lnk[size] = '\0';
+			char *lnk = xmemdup(buf, size);
 			mkdir_p(path, 0777);
 			unlink(path);
 			symlink(lnk, path);
@@ -872,14 +866,9 @@ static int read_merge_config(const char *var, const char *value)
 		if (!strncmp(fn->name, name, namelen) && !fn->name[namelen])
 			break;
 	if (!fn) {
-		char *namebuf;
 		fn = xcalloc(1, sizeof(struct ll_merge_driver));
-		namebuf = xmalloc(namelen + 1);
-		memcpy(namebuf, name, namelen);
-		namebuf[namelen] = 0;
-		fn->name = namebuf;
+		fn->name = xmemdup(name, namelen);
 		fn->fn = ll_ext_merge;
-		fn->next = NULL;
 		*ll_user_merge_tail = fn;
 		ll_user_merge_tail = &(fn->next);
 	}
diff --git a/refs.c b/refs.c
index 7fb3350..64363ce 100644
--- a/refs.c
+++ b/refs.c
@@ -1246,15 +1246,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 static char *ref_msg(const char *line, const char *endp)
 {
 	const char *ep;
-	char *msg;
-
 	line += 82;
-	for (ep = line; ep < endp && *ep != '\n'; ep++)
-		;
-	msg = xmalloc(ep - line + 1);
-	memcpy(msg, line, ep - line);
-	msg[ep - line] = 0;
-	return msg;
+	ep = memchr(line, '\n', endp - line);
+	if (!ep)
+		ep = endp;
+	return xmemdup(line, ep - line);
 }
 
 int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
diff --git a/sha1_file.c b/sha1_file.c
index 59325d4..135bc02 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1491,11 +1491,8 @@ found_cache_entry:
 		ent->lru.next->prev = ent->lru.prev;
 		ent->lru.prev->next = ent->lru.next;
 		delta_base_cached -= ent->size;
-	}
-	else {
-		ret = xmalloc(ent->size + 1);
-		memcpy(ret, ent->data, ent->size);
-		((char *)ret)[ent->size] = 0;
+	} else {
+		ret = xmemdup(ent->data, ent->size);
 	}
 	*type = ent->type;
 	*base_size = ent->size;
@@ -1872,12 +1869,9 @@ void *read_sha1_file(const unsigned char *sha1, enum object_type *type,
 
 	co = find_cached_object(sha1);
 	if (co) {
-		buf = xmalloc(co->size + 1);
-		memcpy(buf, co->buf, co->size);
-		((char*)buf)[co->size] = 0;
 		*type = co->type;
 		*size = co->size;
-		return buf;
+		return xmemdup(co->buf, co->size);
 	}
 
 	buf = read_packed_sha1(sha1, type, size);
diff --git a/tag.c b/tag.c
index bbacd59..f82a668 100644
--- a/tag.c
+++ b/tag.c
@@ -68,9 +68,7 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 	memcpy(type, type_line + 5, typelen);
 	type[typelen] = '\0';
 	taglen = sig_line - tag_line - strlen("tag \n");
-	item->tag = xmalloc(taglen + 1);
-	memcpy(item->tag, tag_line + 4, taglen);
-	item->tag[taglen] = '\0';
+	item->tag = xmemdup(tag_line + 4, taglen);
 
 	if (!strcmp(type, blob_type)) {
 		item->tagged = &lookup_blob(sha1)->object;
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH 1/2] Add xmemdup that duplicates a block of memory, and NUL terminates it.
From: Pierre Habouzit @ 2007-09-15 21:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20070917161113.GB460@artemis.corp>

Implement xstrndup using it.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-compat-util.h |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 1bfbdeb..429b0a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -211,19 +211,20 @@ static inline void *xmalloc(size_t size)
 	return ret;
 }
 
-static inline char *xstrndup(const char *str, size_t len)
+static inline void *xmemdup(const void *data, size_t len)
 {
-	char *p;
-
-	p = memchr(str, '\0', len);
-	if (p)
-		len = p - str;
-	p = xmalloc(len + 1);
-	memcpy(p, str, len);
+	char *p = xmalloc(len + 1);
+	memcpy(p, data, len);
 	p[len] = '\0';
 	return p;
 }
 
+static inline char *xstrndup(const char *str, size_t len)
+{
+	char *p = memchr(str, '\0', len);
+	return xmemdup(str, p ? p - str : len);
+}
+
 static inline void *xrealloc(void *ptr, size_t size)
 {
 	void *ret = realloc(ptr, size);
-- 
1.5.3.1

^ permalink raw reply related

* xmemdup patches
From: Pierre Habouzit @ 2007-09-17 16:11 UTC (permalink / raw)
  To: Git ML

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

  I noticed a lot of places in git's code use code like:

  char *res;

  ...

  res = xmalloc(len + 1);
  memcpy(res, src, len);
  res[len] = '\0';
  return res;

  I've added a "xmemdup" function that duplicates a portion of memory,
also adding an extra NUL after the end of the buffer. There was a
xstrndup already, doing almost the same, except that it worked like
strndup, meaning that it duplicates the memory areay up to len or the
first embeded NUL. The extra scan costs, and is often not necessary (as
we want to extract a token from a buffer we just validated e.g.).

  There were 41 of those places.


  I'm not a huge fan of "xmemdup" as I would not have supposed that a
function called like that would add the extra NUL, so I'm 100% okay with
someone coming up with any better name.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] git-merge: add option --no-ff
From: Chris Shoemaker @ 2007-09-17 16:07 UTC (permalink / raw)
  To: Lars Hjemli
  Cc: Johannes Schindelin, Andreas Ericsson, Junio C Hamano, git,
	Eric Wong
In-Reply-To: <8c5c35580709170712v2f5df7b1w8fa0377b69f24988@mail.gmail.com>

On Mon, Sep 17, 2007 at 04:12:56PM +0200, Lars Hjemli wrote:
> On 9/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > But then, I do not use svn branches here, and that might be the problem?
> 
> Probably. The case I'm trying to solve is:
>   -git-svn branch A is merged into git-svn branch B
>   -A is a fast-forward of B

Ah, now I see what you mean.  But, IIUC, if you want to dcommit your
merge, you should treat it the way svn treats it, with git-merge
--squash.  Then, dcommit won't be confused about the branch you're
committing to.

-chris

> 
> This might look unrealistic, but it happened to me today when I wanted
> to merge a feature-branch into a relase-branch. The release-branch had
> previously been merged into the feature-branch (to get a few
> bugfixes), but the release-branch had not changed since this merge. So
> when merging the feature-branch into the release-branch it just
> fast-forwarded, leaving me with an 'un-dcomittable' release-branch. I
> obviously could have done the merge in subversion (haha!), but doing
> it in git preserves the correct history.
> 
> Btw: I have redone the merge with --no-ff, and dcommit then worked
> like a charm ;-)
> 
> -- 
> larsh
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* git-send-email creates duplicate Message-Id's
From: Adrian Bunk @ 2007-09-17 15:59 UTC (permalink / raw)
  To: Avi Kivity; +Cc: linux-kernel, git
In-Reply-To: <11900179463203-git-send-email-avi@qumranet.com>

The following might be a bug in git-send-email (git maintainers Cc'ed
and KVM list removed from Cc): 

Patch 54 got the same Message-Id as patch 61 and patch 89 got the same 
Message-Id as patch 104.

That's not legal, and users who automatically filter duplicate emails 
based on the Message-Id will not see two of the patches.

The emails are:
http://marc.info/?l=linux-kernel&m=119002061330270&w=2
http://marc.info/?l=linux-kernel&m=119002059626434&w=2
http://marc.info/?l=linux-kernel&m=119002060011801&w=2
http://marc.info/?l=linux-kernel&m=119002060318915&w=2

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply

* Re: metastore
From: Randal L. Schwartz @ 2007-09-17 15:32 UTC (permalink / raw)
  To: Francis Moreau; +Cc: Grzegorz Kulewski, martin f krafft, git
In-Reply-To: <38b2ab8a0709170604u8f4474fyd990f61fb250bc93@mail.gmail.com>

>>>>> "Francis" == Francis Moreau <francis.moro@gmail.com> writes:

Francis> Interesting. Could you show us what this makefile actually looks ?

In fact, I wrote a magazine article about it. :)

http://www.stonehenge.com/merlyn/LinuxMag/col38.html

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] git-merge: add option --no-ff
From: Lars Hjemli @ 2007-09-17 15:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Andreas Ericsson, Junio C Hamano, git, Eric Wong
In-Reply-To: <Pine.LNX.4.64.0709171603090.28586@racer.site>

On 9/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 17 Sep 2007, Lars Hjemli wrote:
>
> > On 9/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > But then, I do not use svn branches here, and that might be the problem?
> >
> > Probably. The case I'm trying to solve is:
> >   -git-svn branch A is merged into git-svn branch B
> >   -A is a fast-forward of B
> >
> > This might look unrealistic, but it happened to me today when I wanted
> > to merge a feature-branch into a relase-branch. The release-branch had
> > previously been merged into the feature-branch (to get a few
> > bugfixes), but the release-branch had not changed since this merge. So
> > when merging the feature-branch into the release-branch it just
> > fast-forwarded, leaving me with an 'un-dcomittable' release-branch. I
> > obviously could have done the merge in subversion (haha!), but doing
> > it in git preserves the correct history.
> >
> > Btw: I have redone the merge with --no-ff, and dcommit then worked
> > like a charm ;-)
>
> Yep, I can see that now.
>
> But maybe there is a better method to detect the latest svn id, by not
> only looking up the svn ids, but making sure that they come from the
> current branch?

Actually, I looked into this last week (my --upstream rants), and I
guess git-svn could use the --track information in .git/config (if
present) as a sanity check when resolving the upstream. But this would
still make the subversion history look like crap after a fast-forward
merge of the kind I was messing with today. It was logically a merge,
but if dcommit had worked 'correctly' it would have created ~150 new
revisions in the release-branch instead of the single merge commit.

> (I'm happily unaware of git-svn's internals, so that might not be
> feasible... But I think that it might be worth fixing that for the git-svn
> idiot like me, since I would never guess that I have to specify --no-ff
> when working on branches that come from git-svn...)

In the normal cases there is no need for --no-ff, only in degenerated
cases like the one I stumbled upon today ;-)

I'll resend the patch with a link from merge-options.txt to
git-svn.txt and try to describe (in git-svn.txt) when to use --no-ff.

-- 
larsh

^ permalink raw reply

* Re: [PATCH] git-merge: add option --no-ff
From: Johannes Schindelin @ 2007-09-17 15:05 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Andreas Ericsson, Junio C Hamano, git, Eric Wong
In-Reply-To: <8c5c35580709170712v2f5df7b1w8fa0377b69f24988@mail.gmail.com>

Hi,

On Mon, 17 Sep 2007, Lars Hjemli wrote:

> On 9/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > But then, I do not use svn branches here, and that might be the problem?
> 
> Probably. The case I'm trying to solve is:
>   -git-svn branch A is merged into git-svn branch B
>   -A is a fast-forward of B
> 
> This might look unrealistic, but it happened to me today when I wanted
> to merge a feature-branch into a relase-branch. The release-branch had
> previously been merged into the feature-branch (to get a few
> bugfixes), but the release-branch had not changed since this merge. So
> when merging the feature-branch into the release-branch it just
> fast-forwarded, leaving me with an 'un-dcomittable' release-branch. I
> obviously could have done the merge in subversion (haha!), but doing
> it in git preserves the correct history.
> 
> Btw: I have redone the merge with --no-ff, and dcommit then worked
> like a charm ;-)

Yep, I can see that now.

But maybe there is a better method to detect the latest svn id, by not 
only looking up the svn ids, but making sure that they come from the 
current branch?

(I'm happily unaware of git-svn's internals, so that might not be 
feasible... But I think that it might be worth fixing that for the git-svn 
idiot like me, since I would never guess that I have to specify --no-ff 
when working on branches that come from git-svn...)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace
From: J. Bruce Fields @ 2007-09-17 15:02 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: Junio C Hamano, git
In-Reply-To: <m3myvlv0m0.fsf@maximus.localdomain>

On Mon, Sep 17, 2007 at 04:16:07PM +0200, Krzysztof Halasa wrote:
> "J. Bruce Fields" <bfields@citi.umich.edu> writes:
> 
> > +test_expect_success \
> > +	'8 or more consecutive initial spaces' \
> > +	'checkstrip 4'
> 
> It may be valid, some projects use tabs for indentation and spaces
> for alignment, e.g.:

Yeah, I know.  I was hoping that the stripspace behavior was already
specific enough to the linux-kernel style that we could just assume that 
it was only used by developers on projects with the same style.  I agree
that I was wrong--apologies.

--b.

^ permalink raw reply

* Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace
From: Krzysztof Halasa @ 2007-09-17 14:16 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Junio C Hamano, git
In-Reply-To: <11899829421064-git-send-email-bfields@citi.umich.edu>

"J. Bruce Fields" <bfields@citi.umich.edu> writes:

> +test_expect_success \
> +	'8 or more consecutive initial spaces' \
> +	'checkstrip 4'

It may be valid, some projects use tabs for indentation and spaces
for alignment, e.g.:

	if (cond && (cond1 ||
	             cond2))
		...

The second line is actually:
	             cond2))
<TAB--->SSSSSSSSSSSSScond2))
where 'S' means space.

This is the only way to write code which display correctly with
different tab sizes.

With tab = 4 spaces it would be expanded to:
    if (cond && (cond1 ||
                 cond2))
        ...
I.e., it would be still fine.


Most of the formating tools probably can't do it automatically.
-- 
Krzysztof Halasa

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox