Git development
 help / color / mirror / Atom feed
* Re: git bundle format
From: Junio C Hamano @ 2012-11-26 20:38 UTC (permalink / raw)
  To: Pyeron, Jason J CTR (US); +Cc: git@vger.kernel.org
In-Reply-To: <871B6C10EBEFE342A772D1159D13208537ABF5AB@umechphj.easf.csd.disa.mil>

"Pyeron, Jason J CTR (US)" <jason.j.pyeron.ctr@mail.mil> writes:

> In this situation we should assume that the bundle does not have
> any content which is already in the public repository, that is it
> has the minimum data to make it pass a git bundle verify from the
> public repositories point of view. We would then take the bundle
> and pipe it though the "git-bundle2text" program which would
> result in a "human" inspectable format as opposed to the packed
> format[2]. The security reviewer would then see all the
> information being released and with the help of the public
> repository see how the data changes the repository.

The bundle file is a thinly wrapped packfile, with extra information
that tells what objects in the bundle are the tips of histories and
what objects the repository the bundle gets unbundled has to have.
So your "git-bundle2text" would likely to involve fetching from the
bundle and inspecting the resulting history and the working tree
files.

^ permalink raw reply

* Re: Ignoring boring lines(that do not contain information) in git diff
From: Peter Oberndorfer @ 2012-11-26 20:35 UTC (permalink / raw)
  To: git
In-Reply-To: <507302DC.4030207@arcor.de>

On 2012-10-08 18:44, Peter Oberndorfer wrote:
> Hi,
>
> is there a way to tell git diff about lines that are uninteresting?
> I mean lines which do not contain a lot of information and
> appear several times in pre and post image.
>
> For example whitespace or language dependent stuff like.
> {
> }
> END_IF;
> END_FOR;
> end sub
>
> I have seen diffs that containing 2 interesting hunks splitted by such boring lines.
> (I have attached a anonymized version of a real world example where this happens)
>
> I think the diff would be clearer when this boring line was added to the surrounding hunks.
> I already tried patience diff but in my test case it changed nothing.
> I am using git 1.7.10.
>

Hi,

does anybody have a idea if this is possible?
Or some comments if they would find such a feature useful?

Greetings Peter


example_diff_boring_split.diff

diff --git a/Source/Frobble/Blabber.txt b/Source/Frobble/Blabber.txt
index 87ccddb..627bc3e 100644
--- a/Source/Frobble/Blabber.txt
+++ b/Source/Frobble/Blabber.txt
@@ -138,73 +138,74 @@ END_VAR
-         //frobble immediately if immediately flag is set
-         IF bImmediately AND NOT Array[i].bDisabled THEN
-            aFrobble(i, Entry);
+         IF Entry.bBlah THEN
+               Alarm.Alarm  := SomeAlarm;
+         ELSE
+               Alarm := Entry;
          END_IF;
-         // signal if frobble count has changed
-         iChanged := iChanged + 1;
-         EXIT;
+         IF Array[i].Alarm = Alarm THEN
+            //do not brabble if alarm is gobbled
+               EXIT;
+         END_IF;
       END_IF;
-   END_FOR;
-ELSE
-   aExample(Name := 'aaa',
-            ID1 := 1);
-END_IF;
+   ELSE
+      //entry not found, adding

^ permalink raw reply related

* Re: git-gui: textconv not used on unstaged files
From: Peter Oberndorfer @ 2012-11-26 20:28 UTC (permalink / raw)
  To: git
In-Reply-To: <5088347F.50503@arcor.de>

On 2012-10-24 20:33, Peter Oberndorfer wrote:
> Hi,
>
> i am using a textconv filter to display .doc files as plain text.
> It seems git gui does not use this textconv filter for displaying new unstaged files
> (other files? = _O)
> It seems diff.tcl start_show_diff calls show_other_diff because of this.
> This manually loads the file and does not care about textconv filters.
>
> Is this manual loading really necessary or can't we just ask git?
> If it is can it be modified to use the textconv filter?
>
Does anybody have a idea which git command
would output the diff of a untracked file against /dev/null?
So I can show the textconved version of the file in git gui.
(and not reinvent the code to apply textconv already in git)

Thanks,
Greetings Peter

>
> .gitattributes:
> *.doc    diff=astextplain
>
> gitconfig:
> [diff "astextplain"]
>     textconv = astextplain

^ permalink raw reply

* difftool -d symlinks, under what conditions
From: Matt McClure @ 2012-11-26 20:23 UTC (permalink / raw)
  To: git

I'm finding the behavior of `git difftool -d` surprising. It seems that it
only uses symlinks to the working copy for files that are modified in the
working copy since the most recent commit. I would have expected it to use
symlinks for all files whose version under comparison is the working copy
version, regardless of whether the working copy differs from the HEAD.

I'm using

    $ git --version
    git version 1.8.0

on a Mac from Homebrew.

--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure

^ permalink raw reply

* Re: git bundle format
From: Felipe Contreras @ 2012-11-26 20:20 UTC (permalink / raw)
  To: Pyeron, Jason J CTR (US); +Cc: git@vger.kernel.org
In-Reply-To: <871B6C10EBEFE342A772D1159D13208537ABF5AB@umechphj.easf.csd.disa.mil>

On Mon, Nov 26, 2012 at 8:24 PM, Pyeron, Jason J CTR (US)
<jason.j.pyeron.ctr@mail.mil> wrote:
> I may need to be nudged in a better direction, but please try to understand my intentions.
>
> I am facing a situation where I would like to use git bundle but at the same time inspect the contents to prevent a spillage[1].
>
> Given we have a public repository which was cloned on to a secret development repository. Now the developers do some work which should not be sensitive in any way and commit and push it to the secret repository.
>
> Now they want to release it out to the public. The current process is to review the text files to ensure that there is no "secret" sauce in there and then approve its release. This current process ignores the change tracking and all non-content is lost.
>
>
> In this situation we should assume that the bundle does not have any content which is already in the public repository, that is it has the minimum data to make it pass a git bundle verify from the public repositories point of view. We would then take the bundle and pipe it though the "git-bundle2text" program which would result in a "human" inspectable format as opposed to the packed format[2]. The security reviewer would then see all the information being released and with the help of the public repository see how the data changes the repository.
>
> Am I barking up the right tree?

Have you tried 'git fast-export'? The output is definitely not human
inspectable, but should be relatively easy to parse to generate such a
format. And instead of 'git bundle unbundle' you could use 'git
fast-import'. or simply do the conversion in your script.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: gitpacker progress report and a question
From: Felipe Contreras @ 2012-11-26 20:07 UTC (permalink / raw)
  To: esr; +Cc: git
In-Reply-To: <20121115212818.GA21558@thyrsus.com>

On Thu, Nov 15, 2012 at 10:28 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Some days ago I reported that I was attempting to write a tool that could
> (a) take a git repo and unpack it into a tarball sequence plus a metadata log,
> (b) reverse that operation, packing a tarball and log sequence into a repo.
>
> Thanks in part to advice by Andreas Schwab and in part to looking at the
> text of the p4 import script, this effort has succeeded.  A proof of
> concept is enclosed.  It isn't documented yet, and has not been tested
> on a repository with branches or merges in the history, but I am confident
> that the distance from here to a finished and tested tool is short.
>
> The immediate intended use is for importing older projects that are
> available only as sequences of release tarballs, but there are other
> sorts of repository surgery that would become easier using it.
>
> I'm still looking for a better name for it and would welcome suggestions.
>
> Before I do much further work, I need to determine how this will be shipped.
> I see two possibilities: either I ship it as a small standalone project,
> or it becomes a git subcommand shipped with the git suite. How I document
> it and set up its tests would differ between these two cases.

Please look at Documentation/SubmittingPatches, you should send
patches in inline format, preferably with 'git format-patch -M', and
preferably with 'git send-email' (in which case you don't need
format-patch), otherwise people will have trouble reviewing, or miss
it completely (as it was the case for me).

I have many comments, but I'll wait until you send the patch inlined,
I'll just address these:

1) I tried it, and it doesn't seem to import (pack?) are repository
with sub-directories in it

2) Using 'git fast-import' is probably simpler, and more efficient

Here is a proof of concept I wrote in ruby that is half the size, and
seems to implement the same functionality. The format is exactly the
same, but I think it should be modified to be more efficient.

Cheers.

>From eb3c34699d7f5d4eec4f088344659b8d9b6a07ea Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Mon, 26 Nov 2012 20:48:38 +0100
Subject: [PATCH] Add new git-weave tool

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/weave/git-weave | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)
 create mode 100755 contrib/weave/git-weave

diff --git a/contrib/weave/git-weave b/contrib/weave/git-weave
new file mode 100755
index 0000000..3106121
--- /dev/null
+++ b/contrib/weave/git-weave
@@ -0,0 +1,166 @@
+#!/usr/bin/env ruby
+
+require 'optparse'
+require 'find'
+require 'fileutils'
+
+def export(indir = '.', out = STDOUT)
+  open(File.join(indir, 'log')).each("\n.\n") do |data|
+
+    @msg = nil
+    @parents = []
+
+    data.chomp(".\n").each_line do |l|
+      if not @msg
+        case l
+        when /^commit (.+)$/
+          @id = $1
+        when /^author (.+)$/
+          @author = $1
+        when /^committer (.+)$/
+          @committer = $1
+        when /^parent (.+)$/
+          @parents << $1
+        when /^$/
+          @msg = ""
+        end
+      else
+        @msg << l
+      end
+    end
+
+    out.puts "commit refs/heads/master"
+    out.puts "mark :#{@id}"
+    out.puts "author #{@author}"
+    out.puts "committer #{@committer}"
+    out.puts "data #{@msg.bytesize}"
+    out.puts @msg
+
+    @parents.each_with_index do |p, i|
+      if i == 0
+        out.puts "from :%u" % p
+      else
+        out.puts "merge :%u" % p
+      end
+    end
+
+    # files
+    out.puts 'deleteall'
+    FileUtils.cd(File.join(indir, @id)) do
+      Find.find('.') do |e|
+        next unless File.file?(e)
+        content = File.read(e)
+        filename = e.split(File::SEPARATOR).slice(1..-1).join(File::SEPARATOR)
+        mode = File.executable?(e) ? '100755' : '100644'
+        if File.symlink?(e)
+          mode = '120000'
+          content = File.readlink(e)
+        end
+        out.puts 'M %s inline %s' % [mode, filename]
+        out.puts "data #{content.bytesize}"
+        out.puts content
+      end
+    end
+
+  end
+end
+
+def import(outdir, out)
+  format = 'format:commit %H%nauthor %an <%ae> %ad%ncommitter %cn
<%ce> %cd%nparents %P%n%n%B'
+  cmd = ['git', 'log', '-z', '-s', '--date=raw', '--format=%s' %
format, '--all', '--reverse']
+  commits = {}
+
+  IO.popen(cmd).each_with_index("\0") do |data, i|
+    @msg = nil
+    @parents = []
+    data.chomp("\0").each_line do |l|
+      if not @msg
+        case l
+        when /^commit (.+)$/
+          @id = $1
+        when /^author (.+)$/
+          @author = $1
+        when /^committer (.+)$/
+          @committer = $1
+        when /^parents (.+)$/
+          @parents = $1.split(" ")
+        when /^$/
+          @msg = ""
+        end
+      else
+        @msg << l
+      end
+    end
+
+    num = i + 1
+    commits[@id] = num
+
+    out.puts "commit #{num}"
+    @parents.each do |p|
+      out.puts "parent #{commits[p]}"
+    end
+    out.puts "author #{@author}"
+    out.puts "committer #{@committer}"
+    out.puts
+    out.puts @msg.gsub(/\n\n+/, "\n") # why?
+    out.puts "."
+
+    wd = File.join(outdir, num.to_s)
+    FileUtils.mkdir_p(wd)
+    system('git', '--work-tree', wd, 'checkout', '-f', '-q', @id)
+  end
+end
+
+def git_pack(indir, outdir)
+  indir = File.absolute_path(indir)
+  system('git', 'init', outdir)
+  FileUtils.cd(outdir) do
+    IO.popen(['git', 'fast-import'], 'w') do |io|
+      export(indir, io)
+    end
+    system('git', 'reset', '--hard')
+  end
+end
+
+def git_unpack(indir, outdir)
+  begin
+    FileUtils.mkdir_p(outdir)
+    log = File.open(File.join(outdir, 'log'), 'w')
+    ENV['GIT_DIR'] = File.join(indir, '.git')
+    import(outdir, log)
+  ensure
+    system('git', 'symbolic-ref', 'HEAD', 'refs/heads/master')
+    ENV.delete('GIT_DIR')
+    log.close if log
+  end
+end
+
+$indir = '.'
+
+begin
+  OptionParser.new do |opts|
+    opts.on('-x') do
+      $mode = 'unpack'
+    end
+    opts.on('-c') do
+      $mode = 'pack'
+    end
+    opts.on('-o', '--outdir DIR') do |v|
+      $outdir = v
+    end
+    opts.on('-i', '--indir DIR') do |v|
+      $indir = v
+    end
+  end.parse!
+rescue OptionParser::InvalidOption
+end
+
+$mode = File.exists?(File.join($indir, '.git')) ? 'unpack' : 'pack'
unless $mode
+$outdir = File.join($indir, $mode == 'pack' ? 'packed' : 'unpacked2')
unless $outdir
+
+case $mode
+when 'pack'
+  git_pack($indir, $outdir)
+when 'unpack'
+  git_unpack($indir, $outdir)
+end
-- 
1.8.0

-- 
Felipe Contreras

^ permalink raw reply related

* Re: [PATCH] fast-export: Allow pruned-references in mark file
From: Antoine Pelisse @ 2012-11-26 20:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vhaoctg6i.fsf@alter.siamese.dyndns.org>

> I am not sure I follow the above, but anyway, I think the patch does
> is safe because (1) future "fast-export" will not refer to these
> pruned objects in its output (we have decided that these pruned
> objects are not used anywhere in the history so nobody will refer to
> them) and (2) we still need to increment the id number so that later
> objects in the marks file get assigned the same id number as they
> were assigned originally (otherwise we will not name these objects
> consistently when we later talk about them).

I fully agree on (1), not so much on (2) though.

I have the following behavior using my patch and running that script
that doesn't look correct.

echo "Working scenario"
git init test &&
(cd test &&
git commit --allow-empty -m "Commit mark :1" &&
git commit --allow-empty -m "Commit mark :2" &&
git fast-export --export-marks=MARKS master > /dev/null &&
cat MARKS &&
git reset HEAD~1 &&
sleep 1 &&
git reflog expire --all --expire=now &&
git prune --expire=now &&
git commit --allow-empty -m "Commit mark :3" &&
git fast-export --import-marks=MARKS \
  --export-marks=MARKS master > /dev/null &&
cat MARKS) &&
rm -rf test

echo "Non-working scenario"
git init test &&
(cd test &&
git commit --allow-empty -m "Commit mark :1" &&
git commit --allow-empty -m "Commit mark :2" &&
git fast-export --export-marks=MARKS master > /dev/null &&
cat MARKS &&
git reset HEAD~1 &&
sleep 1 &&
git reflog expire --all --expire=now &&
git prune --expire=now &&
git fast-export --import-marks=MARKS \
  --export-marks=MARKS master > /dev/null &&
git commit --allow-empty -m "Commit mark :3" &&
git fast-export --import-marks=MARKS \
  --export-marks=MARKS master > /dev/null &&
cat MARKS) &&
rm -rf test

outputs something like this:
Working scenario
Initialized empty Git repository in /home/antoine/test/.git/
[master (root-commit) 6cf350d] Commit mark :1
[master 8f97f85] Commit mark :2
:1 6cf350d7ecb3dc6573b00f839a6a51625ed28966
:2 8f97f85e1e7badf6a3daf411cf8d1133b00d522e
[master 21cadfd] Commit mark :3
warning: Could not read blob 8f97f85e1e7badf6a3daf411cf8d1133b00d522e
:1 6cf350d7ecb3dc6573b00f839a6a51625ed28966
:3 21cadfd87d90c05ce8770c968e5ed3d072ead4ae
Non-working scenario
Initialized empty Git repository in /home/antoine/test/.git/
[master (root-commit) 5b5f7ec] Commit mark :1
[master b224390] Commit mark :2
:2 b224390daee199644495c15503882eb84df07df5
:1 5b5f7ec77768393aab2a0c2c11b4b8f7773f8678
warning: Could not read blob b224390daee199644495c15503882eb84df07df5
[master 181a774] Commit mark :3
:1 5b5f7ec77768393aab2a0c2c11b4b8f7773f8678
:2 181a7744c6d3428edb01a1adc9df247e9620be5f

Both "commit mark :2" and "commit mark :3" end up being marked :2.
Any tool like git-remote-hg that is using a mapping from mark <-> hg changeset
could then fail.

^ permalink raw reply

* Re: [PATCH] Third try at documenting command integration requirements.
From: Junio C Hamano @ 2012-11-26 20:01 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121126053557.E56434065F@snark.thyrsus.com>

esr@thyrsus.com (Eric S. Raymond) writes:

> This document contains no new policies or proposals; it attempts
> to document established practices and interface requirements.
>
> Signed-off-by: Eric S. Raymond <esr@thyrsus.com>

I'll reword the title (readers of "git log" output 6 months down the
road will not care if this is the third try or the first one) and
tweak things here and there before queuing.

> diff --git a/Documentation/technical/api-command.txt b/Documentation/technical/api-command.txt
> new file mode 100644
> index 0000000..c1c1afb
> --- /dev/null
> +++ b/Documentation/technical/api-command.txt
> @@ -0,0 +1,91 @@
> += Integrating new subcommands =
> +
> +This is how-to documentation for people who want to add extension
> +commands to git.  It should be read alongside api-builtin.txt.
> +
> +== Runtime environment ==
> +
> +git subcommands are standalone executables that live in the git
> +execution directory, normally /usr/lib/git-core.  The git executable itself
> +is a thin wrapper that sets GIT_DIR and passes command-line arguments
> +to the subcommand.

    $ echo >$HOME/bin/git-showenv '#!/bin/sh
    exec env'
    $ chmod +x $HOME/bin/git-showenv
    $ git showenv | grep GIT_

gives me emptyness.  I rewrote the above to:

    git subcommands are standalone executables that live in the git exec
    path, normally /usr/lib/git-core.  The git executable itself is a
    thin wrapper that knows where the subcommands live, and runs them by
    passing command-line arguments to them.

FYI, a builtin command _can_ ask the git wrapper to set up the
execution environment by setting RUN_SETUP bit in its cmd_struct
entry, but it is not done by default.

> +== Implementation languages ==
> +
> +Most subcommands are written in C or shell.  A few are written in
> +Perl.  A tiny minority are written in Python.
> +
> +While we strongly encourage coding in portable C for portability, these
> +specific scripting languages are also acceptable. We won't accept more
> +without a very strong technical case, as we don't want to broaden the
> +git suite's required dependencies.
> +
> +Python is fine for import utilities, surgical tools, remote helpers
> +and other code at the edges of the git suite - but it should not yet
> +be used for core functions. This may change in the future; the problem
> +is that we need better Python integration in the git Windows installer
> +before we can be confident people in that environment won't
> +experience an unacceptably large loss of capability.

As Felipe and others said in the discussion, Python is not *that*
special over other languages (and I think we have a Go in contrib/).

I rewrote the above to:

    Most subcommands are written in C or shell.  A few are written in
    Perl.

    While we strongly encourage coding in portable C for portability,
    these specific scripting languages are also acceptable.  We won't
    accept more without a very strong technical case, as we don't want
    to broaden the git suite's required dependencies.  Import utilities,
    surgical tools, remote helpers and other code at the edges of the
    git suite are more lenient and we allow Python (and even Tcl/tk),
    but they should not be used for core functions.

    This may change in the future.  Especially Python is not allowed in
    core because we need better Python integration in the git Windows
    installer before we can be confident people in that environment
    won't experience an unacceptably large loss of capability.

> +C commands are normally written as single modules, named after the
> +command, that link a collection of functions called libgit.  Thus,
> +your command 'git-foo' would normally be implemented as a single
> +"git-foo.c"; this organization makes it easy for people reading the

    "git-foo.c" (or "builtin/foo.c" if it is to be linked to the main
    binary);

> +4. If your command has any dependency on a a particular version of
> +your language, document it in the INSTALL file.

    s/a a/a/;

> +6. When your patch is merged, remind the maintainer to add something
> +about it in the RelNotes file.

    6. Give the maintainer a one paragraph to include in the RelNotes
    file to describe the new feature; a good place to do so is in the
    cover letter [PATCH 0/n].

Thanks.

^ permalink raw reply

* Re: [PATCH v3] send-email: avoid questions when user has an ident
From: Junio C Hamano @ 2012-11-26 19:35 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Jeff King, Thomas Rast, Christian Couder, Stephen Boyd,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <1353755779-32548-1-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> 3) Full name + fqdm
>
>   Who should the emails appear to be from?
>   [Felipe Contreras <felipec@nysa.felipec.org>]
> ...
> This is bad, because we will try with an address such as
> 'felipec@nysa.felipec.org', which is most likely not what the user
> wants, but the user will get warned by default (confirm=auto), and
> if not, most likely the sending won't work, which the user would
> readily note and fix.

OK, sounds sensible.

> Changes since v2:
>
>  * Merge the relevant tests by Jeff King (the rest of the tests might be
>    useful, but they belong in a separate patch series)

Thanks. Queued.

^ permalink raw reply

* RE: git bundle format
From: Pyeron, Jason J CTR (US) @ 2012-11-26 19:31 UTC (permalink / raw)
  To: Pyeron, Jason J CTR (US), git@vger.kernel.org
In-Reply-To: <871B6C10EBEFE342A772D1159D13208537ABF5AB@umechphj.easf.csd.disa.mil>

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

Left off a citation to an old thread.

> -----Original Message-----
> From: Pyeron, Jason J CTR (US)
> Sent: Monday, November 26, 2012 2:25 PM
> 
> I may need to be nudged in a better direction, but please try to
> understand my intentions.
> 
> I am facing a situation where I would like to use git bundle but at the
> same time inspect the contents to prevent a spillage[1].
> 
> Given we have a public repository which was cloned on to a secret
> development repository. Now the developers do some work which should
> not be sensitive in any way and commit and push it to the secret
> repository.
> 
> Now they want to release it out to the public. The current process is
> to review the text files to ensure that there is no "secret" sauce in
> there and then approve its release. This current process ignores the
> change tracking and all non-content is lost.
> 
> 
> In this situation we should assume that the bundle does not have any
> content which is already in the public repository, that is it has the
> minimum data to make it pass a git bundle verify from the public
> repositories point of view. We would then take the bundle and pipe it
> though the "git-bundle2text" program which would result in a "human"
> inspectable format
[3]
> as opposed to the packed format[2]. The security
> reviewer would then see all the information being released and with the
> help of the public repository see how the data changes the repository.
> 
> Am I barking up the right tree?
> 
> 
> 1: http://en.wikipedia.org/wiki/Spillage_of_Classified_Information
> 2: http://git-scm.com/book/ch9-4.html
3: http://git.661346.n2.nabble.com/How-to-extract-files-out-of-a-quot-git-bundle-quot-no-matter-what-td1679188.html

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]

^ permalink raw reply

* git bundle format
From: Pyeron, Jason J CTR (US) @ 2012-11-26 19:24 UTC (permalink / raw)
  To: git@vger.kernel.org

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

I may need to be nudged in a better direction, but please try to understand my intentions.

I am facing a situation where I would like to use git bundle but at the same time inspect the contents to prevent a spillage[1].

Given we have a public repository which was cloned on to a secret development repository. Now the developers do some work which should not be sensitive in any way and commit and push it to the secret repository.

Now they want to release it out to the public. The current process is to review the text files to ensure that there is no "secret" sauce in there and then approve its release. This current process ignores the change tracking and all non-content is lost.


In this situation we should assume that the bundle does not have any content which is already in the public repository, that is it has the minimum data to make it pass a git bundle verify from the public repositories point of view. We would then take the bundle and pipe it though the "git-bundle2text" program which would result in a "human" inspectable format as opposed to the packed format[2]. The security reviewer would then see all the information being released and with the help of the public repository see how the data changes the repository.

Am I barking up the right tree?


1: http://en.wikipedia.org/wiki/Spillage_of_Classified_Information
2: http://git-scm.com/book/ch9-4.html


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]

^ permalink raw reply

* [PATCH] Fix typo in remote set-head usage
From: Antoine Pelisse @ 2012-11-26 19:21 UTC (permalink / raw)
  To: Junio C Hamano, git; +Cc: Jiang Xin, Antoine Pelisse
In-Reply-To: <7vwqx8rzzf.fsf@alter.siamese.dyndns.org>

parenthesis are not matching in `builtin_remote_sethead_usage`
as a square bracket is closing something never opened.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 builtin/remote.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index a5a4b23..937484d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -39,7 +39,7 @@ static const char * const builtin_remote_rm_usage[] = {
 };
 
 static const char * const builtin_remote_sethead_usage[] = {
-	N_("git remote set-head <name> (-a | -d | <branch>])"),
+	N_("git remote set-head <name> (-a | -d | <branch>)"),
 	NULL
 };
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-11-26 19:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Jeff King, Jonathan Nieder, git, Max Horn,
	Sverre Rabbelier, Brandon Casey, Brandon Casey, Ilari Liusvaara,
	Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <7vd2z0tfhz.fsf@alter.siamese.dyndns.org>

On Mon, Nov 26, 2012 at 6:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> If you changed your stance on the patch Sverre and I sent to fix this, we
>> could get a non-partial fix for this.
>
> This is long time ago so I may be misremembering the details, but I
> thought the original patch was (ab)using object flags to mark "this
> was explicitly asked for, even though some other range operation may
> have marked it uninteresting".  Because it predated the introduction
> of the rev_cmdline_info mechanism to record what was mentioned on
> the command line separately from what objects are uninteresting
> (i.e. object flags), it may have been one convenient way to record
> this information, but it still looked unnecessarily ugly hack to me,
> in that it allocated scarce object flag bits to represent a narrow
> special case (iirc, only a freestanding "A" on the command line but
> not "A" spelled in "B..A", or something), making it more expensive
> to record other kinds of command line information in a way
> consistent with the approach chosen (we do not want to waste object
> flag bits in order to record "this was right hand side tip of the
> symmetric difference range" and such).
>
> If you are calling "do not waste object flags to represent one
> special case among endless number of possibilities, as it will make
> it impossible to extend it" my stance, that hasn't changed.

The problem with those patches is that they were doing many things at
the same time.

You are correct that one of the problems being solved was the fact
that we wanted to differentiate B from A in B..A independently of the
object, because it might have been referenced by ^C. My latest patch
series deals with that by using rev cmdline_info.

But there's another problem that series tried to fix: weather or not A
was exported by fast-export, which is not strictly the same as SHOWN.

This becomes a non-issue if my patch series is applied because it
properly identifies when an object has been marked or not. But it's
not when marks are not used.

For example:

% git branch A v1
% git branch B v0
% git branch C v0
% git branch D v1
% git fast-export B..A ^C D

A would be updated through a 'commit refs/heads/A' command, D would be
updated through 'reset refs/heads/D'.

But what if C points to v1? The code will assume A will be exported,
and it will be skipped, and there will be only one reset: 'reset
refs/heads/D'. Either way it doesn't matter, because the reset would
be to mark :0, so even if there was a 'reset refs/heads/A' (because A
was never exported), a mark :0 would be useless.

When marks are used my patch fixes the problem because it doesn't care
if A was exporeted or not; by knowing it was marked, it knows it was
never intended to be exported, so we get resets for both A and D, with
real marks.

> We added rev_cmdline_info since then so that we can tell what refs
> were given from the command line in what way, and I thought that we
> applied a patch from Sverre that uses it instead of the object
> flags.  Am I misremembering things?

No, the patch from Sverre was never merged.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 5/7] push: require force for refs under refs/tags/
From: Junio C Hamano @ 2012-11-26 18:57 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <1353644515-17349-6-git-send-email-chris@rorvick.com>

Chris Rorvick <chris@rorvick.com> writes:

> diff --git a/remote.c b/remote.c
> index 4a6f822..012b52f 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>  		 *
>  		 * (1) if the old thing does not exist, it is OK.
>  		 *
> -		 * (2) if you do not have the old thing, you are not allowed
> +		 * (2) if the destination is under refs/tags/ you are
> +		 *     not allowed to overwrite it; tags are expected
> +		 *     to be static once created
> +		 *
> +		 * (3) if you do not have the old thing, you are not allowed
>  		 *     to overwrite it; you would not know what you are losing
>  		 *     otherwise.
>  		 *
> -		 * (3) if both new and old are commit-ish, and new is a
> +		 * (4) if both new and old are commit-ish, and new is a
>  		 *     descendant of old, it is OK.
>  		 *
> -		 * (4) regardless of all of the above, removing :B is
> +		 * (5) regardless of all of the above, removing :B is
>  		 *     always allowed.
>  		 */

We may want to reword (0) to make it clear that --force
(and +A:B) can be used to defeat all the other rules.

The updated logic in the patch looks sensible.  Thanks.

> ...
> +test_expect_success 'push requires --force to update lightweight tag' '
> +	mk_test heads/master &&
> +	mk_child child1 &&
> +	mk_child child2 &&
> +	(
> +		cd child1 &&
> +		git tag Tag &&
> +		git push ../child2 Tag &&
> +		git push ../child2 Tag &&
> +		>file1 &&
> +		git add file1 &&
> +		git commit -m "file1" &&
> +		git tag -f Tag &&
> +		test_must_fail git push ../child2 Tag &&
> +		git push --force ../child2 Tag &&
> +		git tag -f Tag &&
> +		test_must_fail git push ../child2 Tag HEAD~ &&
> +		git push --force ../child2 Tag
> +	)
> +'
> +

^ permalink raw reply

* Re: [PATCH 7/7] push: clarify rejection of update to non-commit-ish
From: Junio C Hamano @ 2012-11-26 18:53 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <1353644515-17349-8-git-send-email-chris@rorvick.com>

Chris Rorvick <chris@rorvick.com> writes:

> Pushes must already (by default) update to a commit-ish due the fast-
> forward check in set_ref_status_for_push().  But rejecting for not being
> a fast-forward suggests the situation can be resolved with a merge.
> Flag these updates (i.e., to a blob or a tree) as not forwardable so the
> user is presented with more appropriate advice.
>
> Signed-off-by: Chris Rorvick <chris@rorvick.com>
> ---
>  remote.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/remote.c b/remote.c
> index f5bc4e7..ee0c1e5 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1291,6 +1291,11 @@ static inline int is_forwardable(struct ref* ref)
>  	if (!o || o->type != OBJ_COMMIT)
>  		return 0;
>  
> +	/* new object must be commit-ish */
> +	o = deref_tag(parse_object(ref->new_sha1), NULL, 0);
> +	if (!o || o->type != OBJ_COMMIT)
> +		return 0;
> +

I think the original code used ref_newer() which took commit-ish on
both sides.

With this code, the old must be a commit but new can be a tag that
points at a commit?  Why?

^ permalink raw reply

* Re: [PATCH 1/7] push: return reject reasons via a mask
From: Junio C Hamano @ 2012-11-26 18:43 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <1353644515-17349-2-git-send-email-chris@rorvick.com>

Chris Rorvick <chris@rorvick.com> writes:

> Pass all rejection reasons back from transport_push().  The logic is
> simpler and more flexible with regard to providing useful feedback.
>
> Signed-off-by: Chris Rorvick <chris@rorvick.com>
> ---

Thanks for a reroll.

I do not think they are "masks", by the way.  They are set of flags
(i.e. a bitset).

A bitset is often called "a mask" when it is used to "mask" a subset
of bits in another bitset that has the real information, either to
ignore irrelevant bits or to pick only the relevant bits from the
latter.  And your "reject_mask" is never used as a mask in this
patch---it is the bitset that has the real information and it gets
masked by constant masks like REJECT_NON_FF_HEAD.

In any case, naming it as "reject_mask" is like calling a counter as
"counter_int".  It is more important to name it after its purpose
than after its type, and because this is to record the reasons why
the push was rejected, "rejection_reason" might be a better name for
it.

^ permalink raw reply

* Re: [PATCH] makefile: hide stderr of curl-config test
From: Junio C Hamano @ 2012-11-26 18:30 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: git
In-Reply-To: <1353554397-27162-1-git-send-email-paul.gortmaker@windriver.com>

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> Currently, if you don't have curl installed, you will get
>
>     $ make distclean 2>&1 | grep curl
>     /bin/sh: curl-config: not found
>     /bin/sh: curl-config: not found
>     /bin/sh: curl-config: not found
>     /bin/sh: curl-config: not found
>     /bin/sh: curl-config: not found
>     $
>
> The intent is not to alarm the user, but just to test if there is
> a new enough curl installed.  However, if you look at search engine
> suggested completions, the above "error" messages are confusing
> people into thinking curl is a hard requirement.

Good observation and identification of an issue to tackle.  But why
isn't the patch like this?

 	PROGRAMS += $(REMOTE_CURL_NAMES)
-	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
+	curl_check := $(shell (echo 070908; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
 	ifeq "$(curl_check)" "070908"

Removal of the "reject old libcURL" is logically a separate thing
regardless of the "alarming output from make", and it probably is
better done as a separate step in a two-patch series.  Doing things
that way, when somebody objects to this:

> It wants to ensure curl is newer than 070908.  The oldest
> machine I could find (RHEL 4.6) is 2007 vintage according
> to /proc/version data, and it has curl 070C01.

saying that their installation still cares about older libcURL, we
can still keep the "remove alarming output from make" bit.

>
> The failure here is to mask stderr in the test.  However, since
> the chance of curl being installed, but too old is essentially
> nil, lets just check for existence and drop the ancient version
> threshold check, if for no other reason, than to simplifly the
> parsing of what the makefile is trying to do by humans.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> diff --git a/Makefile b/Makefile
> index 9bc5e40..56f55f6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1573,8 +1573,8 @@ else
>  	REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
>  	PROGRAM_OBJS += http-fetch.o
>  	PROGRAMS += $(REMOTE_CURL_NAMES)
> -	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
> -	ifeq "$(curl_check)" "070908"
> +	curl_check := $(shell curl-config --vernum 2>/dev/null)
> +	ifneq "$(curl_check)" ""
>  		ifndef NO_EXPAT
>  			PROGRAM_OBJS += http-push.o
>  		endif

^ permalink raw reply

* Re: commit gone after merge - how to debug?
From: Igor Lautar @ 2012-11-26 17:01 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Tomas Carnecky, git
In-Reply-To: <vpqa9u4pgew.fsf@grenoble-inp.fr>

On Mon, Nov 26, 2012 at 3:50 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> What's possible is that someone had already merged the branch containing
> "new", got conflicts, and resolved it in favor of "old" somewhere in the
> history of your master branch.

This is exactly what happened. I've actually found a merge of origin
to mirror which reversed the change some time back and was
subsequently merged back to origin later on. Most probably human error
during merge.

Interestingly, this was my first thought as well, but I've must have
overlooked that particular merge the first time.

Anyhow, it sorted now, many thanks for your help,
Igor

^ permalink raw reply

* Re: [PATCH] Fix typo in remote set-head usage
From: Junio C Hamano @ 2012-11-26 18:16 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git, tim.henigan, Jiang Xin
In-Reply-To: <1353851019-27254-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> parenthesis are not matching in `builtin_remote_sethead_usage`
> as a square bracket is closing something never opened.
> ---
> This will also break all translation files, should I also send a patch
> to update them ?

"git grep -A2 -e 'remote set-head <name>' po/" tells me that we
already have both the correct variant and the broken one, and they
are both translated ;-) so I do not think we have much to worry
about the i18n fallout in this case, but thanks anyway for thinking
about it.

You would need to sign off your patch, but otherwise looks good to
me.

Thanks.


>
> Cheers,
> Antoine Pelisse
>
>  builtin/remote.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index a5a4b23..937484d 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -39,7 +39,7 @@ static const char * const builtin_remote_rm_usage[] = {
>  };
>
>  static const char * const builtin_remote_sethead_usage[] = {
> -	N_("git remote set-head <name> (-a | -d | <branch>])"),
> +	N_("git remote set-head <name> (-a | -d | <branch>)"),
>  	NULL
>  };
>
> --
> 1.7.9.5

^ permalink raw reply

* Re: [PATCH 3/5] git-send-email: remove invalid addresses earlier
From: Junio C Hamano @ 2012-11-26 17:02 UTC (permalink / raw)
  To: Krzysztof Mazur
  Cc: git, Felipe Contreras, Andreas Schwab, Felipe Balbi,
	Tomi Valkeinen
In-Reply-To: <1353607932-10436-3-git-send-email-krzysiek@podlesie.net>

Krzysztof Mazur <krzysiek@podlesie.net> writes:

> Some addresses are passed twice to unique_email_list() and invalid addresses
> may be reported twice per send_message. Now we warn about them earlier
> and we also remove invalid addresses.
>
> This also removes using of undefined values for string comparison
> for invalid addresses in cc list processing.
>
> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> ---

I think there are three kinds of address-looking things we deal
with:

 * Possibly invalid but meant for human consumption, e.g.

     Cc: Stable Kernel <stable@k.org> # for v3.5 and upwards

   in the commit log message trailer.

 * Meant to be fed to our MSA, without losing the human readable
   part, e.g.

     Cc: Stable Kernel <stable@k.org>

   in the header of the outgoing message.

 * Without the human-readable part, e.g.

     stable@k.org

   that is returned by extract_valid_address.

My understanding is that our input typically comes from the first
kind and sanitize_address() is meant to massage it into the second
kind.  The last kind is to be used to drive the underlying sendmail
machinery and meant to go in the envelope (this includes message-id
generation).

I do not think send-email adds the first kind (invalid ones) in its
output, even though it reads them from its input and copy them to
its output in the e-mail body part of the payload, but I think it
adds new addresses to the e-mail header part of the payload (that is
what $from, @initial_to, @initial_cc and @bcclist are all about).
We would want to feed them in the third form (i.e. output from
extract-valid-address on them) when driving the underlying sendmail
machinery to place them in the envelope part, but they should be in
the second form when we place them on e-mail header lines.  As far
as I can tell, the resulting code looks correct in this regard.  The
addresses are sanitized into the second form upfront and validated
before they are placed in @initial_to and friends, and we carry the
second form around most of the time, until we call unique_email_list
in send_message to pass them through extract_valid_address to turn
them into the third form to drive the underlying sendmail.

I however found it a bit confusing while reading the callers of
validate_address{,_list} functions, which not just validate (and
warns) but return the ones that pass the test.  Perhaps we would
want a brief comment before validate_address, validate_address_list,
and extract_valid_address{,_or_die} to clarify what they are doing
(especially what they return)?

The result still feels somewhat yucky (the yuckiness comes primarily
from the current code, not from the patch but I am mostly focused on
the result after applying the patch), in that extract-valid-address
that has problem with invalid email addresses will still die when
fed an address that is not "sanitized" first, so any future patch
that adds a new address source may still have to suffer the same
problem the part that dealt with the Cc list suffered (which your
1/5 fixed earlier), but I do not offhand think of a good way to
reorganize them.  We could of course make validate_address() call
sanitize_address(), but that would be mostly redundant since the new
code sanitizes the input upfront.

So overall, looks good to me.  Thanks.

>  git-send-email.perl | 52 +++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 356f99d..5056fdc 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -786,9 +786,11 @@ sub expand_one_alias {
>  }
>  
>  @initial_to = expand_aliases(@initial_to);
> -@initial_to = (map { sanitize_address($_) } @initial_to);
> +@initial_to = validate_address_list(sanitize_address_list(@initial_to));
>  @initial_cc = expand_aliases(@initial_cc);
> +@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
>  @bcclist = expand_aliases(@bcclist);
> +@bcclist = validate_address_list(sanitize_address_list(@bcclist));

^ permalink raw reply

* Re: Long clone time after "done."
From: Uri Moszkowicz @ 2012-11-26 18:06 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <CAMJd5AQOH=uu1c3enr5fES+Bw4FHZuP++J7-aCM2B+f_G3YMtg@mail.gmail.com>

Hi guys,
Any further interest on this scalability problem or should I move on?

Thanks,
Uri

On Thu, Nov 8, 2012 at 5:35 PM, Uri Moszkowicz <uri@4refs.com> wrote:
> I tried on the local disk as well and it didn't help. I managed to
> find a SUSE11 machine and tried it there but no luck so I think we can
> eliminate NFS and OS as significant factors now.
>
> I ran with perf and here's the report:
>
> ESC[31m    69.07%ESC[m              git  /lib64/libc-2.11.1.so
>                                          [.] memcpy
> ESC[31m    12.33%ESC[m              git
> <prefix>/git-1.8.0.rc2.suse11/bin/git                           [.]
> blk_SHA1_Block
> ESC[31m     5.11%ESC[m              git
> <prefix>/zlib/local/lib/libz.so.1.2.5                           [.]
> inflate_fast
> ESC[32m     2.61%ESC[m              git
> <prefix>/zlib/local/lib/libz.so.1.2.5                           [.]
> adler32
> ESC[32m     1.98%ESC[m              git  /lib64/libc-2.11.1.so
>                                          [.] _int_malloc
> ESC[32m     0.86%ESC[m              git  [kernel]
>                                          [k] clear_page_c
>
> Does this help? Machine has 396GB of RAM if it matters.
>
> On Thu, Nov 8, 2012 at 4:33 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Nov 08, 2012 at 04:16:59PM -0600, Uri Moszkowicz wrote:
>>
>>> I ran "git cat-file commit some-tag" for every tag. They seem to be
>>> roughly uniformly distributed between 0s and 2s and about 2/3 of the
>>> time seems to be system. My disk is mounted over NFS so I tried on the
>>> local disk and it didn't make a difference.
>>>
>>> I have only one 1.97GB pack. I ran "git gc --aggressive" before.
>>
>> Ah. NFS. That is almost certainly the source of the problem. Git will
>> aggressively mmap. I would not be surprised to find that RHEL4's NFS
>> implementation is not particularly fast at mmap-ing 2G files, and is
>> spending a bunch of time in the kernel servicing the requests.
>>
>> Aside from upgrading your OS or getting off of NFS, I don't have a lot
>> of advice.  The performance characteristics you are seeing are so
>> grossly off of what is normal that using git is probably going to be
>> painful. Your 2s cat-files should be more like .002s. I don't think
>> there's anything for git to fix here.
>>
>> You could try building with NO_MMAP, which will emulate it with pread.
>> That might fare better under your NFS implementation. Or it might be
>> just as bad.
>>
>> -Peff

^ permalink raw reply

* Re: [RFC/PATCH] Option to revert order of parents in merge commit
From: Junio C Hamano @ 2012-11-26 17:58 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git
In-Reply-To: <20121126124200.GA29859@camk.edu.pl>

Kacper Kornet <draenog@pld-linux.org> writes:

>> Is there any interaction between this "pull --reverse-parents"
>> change and possible conflict resolution when the command stops and
>> asks the user for help?  For example, whom should "--ours" and "-X
>> ours" refer to?  Us, or the upstream?
>
> The change of order of parents happens at the very last moment, so
> "ours" in merge options is local version and "theirs" upstream.

That may be something that wants to go to the proposed commit log
message.  I am neutral on the "feature" (i.e. not against it but not
extremely enthusiastic about it either).

Thanks.

^ permalink raw reply

* Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs
From: Junio C Hamano @ 2012-11-26 17:56 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Felipe Contreras, Jeff King, Jonathan Nieder, git, Max Horn,
	Sverre Rabbelier, Brandon Casey, Brandon Casey, Ilari Liusvaara,
	Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <alpine.DEB.1.00.1211261726260.7256@s15462909.onlinehome-server.info>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> If you changed your stance on the patch Sverre and I sent to fix this, we
> could get a non-partial fix for this.

This is long time ago so I may be misremembering the details, but I
thought the original patch was (ab)using object flags to mark "this
was explicitly asked for, even though some other range operation may
have marked it uninteresting".  Because it predated the introduction
of the rev_cmdline_info mechanism to record what was mentioned on
the command line separately from what objects are uninteresting
(i.e. object flags), it may have been one convenient way to record
this information, but it still looked unnecessarily ugly hack to me,
in that it allocated scarce object flag bits to represent a narrow
special case (iirc, only a freestanding "A" on the command line but
not "A" spelled in "B..A", or something), making it more expensive
to record other kinds of command line information in a way
consistent with the approach chosen (we do not want to waste object
flag bits in order to record "this was right hand side tip of the
symmetric difference range" and such).

If you are calling "do not waste object flags to represent one
special case among endless number of possibilities, as it will make
it impossible to extend it" my stance, that hasn't changed.

We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags.  Am I misremembering things?

^ permalink raw reply

* Re: [PATCH 5/5] git-send-email: allow edit invalid email address
From: Junio C Hamano @ 2012-11-26 17:08 UTC (permalink / raw)
  To: Krzysztof Mazur
  Cc: git, Felipe Contreras, Andreas Schwab, Felipe Balbi,
	Tomi Valkeinen
In-Reply-To: <1353607932-10436-5-git-send-email-krzysiek@podlesie.net>

Krzysztof Mazur <krzysiek@podlesie.net> writes:

> In some cases the user may want to send email with "Cc:" line with
> email address we cannot extract. Now we allow user to extract
> such email address for us.
>
> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> ---
>  git-send-email.perl | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d42dca2..9996735 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
>  
>  sub validate_address {
>  	my $address = shift;
> -	if (!extract_valid_address($address)) {
> +	while (!extract_valid_address($address)) {
>  		print STDERR "error: unable to extract a valid address from: $address\n";
> -		$_ = ask("What to do with this address? ([q]uit|[d]rop): ",
> -			valid_re => qr/^(?:quit|q|drop|d)/i,
> +		$_ = ask("What to do with this address? ([q]uit|[d]rop|[e]dit): ",
> +			valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
>  			default => 'q');
>  		if (/^d/i) {
>  			return undef;
> @@ -862,6 +862,9 @@ sub validate_address {
>  			cleanup_compose_files();
>  			exit(0);
>  		}
> +		$address = ask("Who should the email be sent to (if any)? ",
> +			default => "",
> +			valid_re => qr/\@.*\./, confirm_only => 1);

Not having this new code inside "elsif (/^e/) { }" feels somewhat
sloppy, even though it is not *too* bad.  Also do we know this
function will never be used for addresses other than recipients' (I
gave a cursory look to see what is done to the $sender and it does
not seem to go through this function, tho)?

^ permalink raw reply

* Re: [PATCH] fast-export: Allow pruned-references in mark file
From: Junio C Hamano @ 2012-11-26 17:41 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Felipe Contreras, git
In-Reply-To: <CALWbr2yZpAT=eSahGcGKw5weoz1MjTzbb16pdQndKDFcn_3VJg@mail.gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> On Mon, Nov 26, 2012 at 12:37 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Mon, Nov 26, 2012 at 5:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Is this a safe and sane thing to do, and if so why?  Could you
>>> describe that in the log message here?
>> Why would fast-export try to export something that was pruned? Doesn't
>> that mean it wasn't reachable?
>
> Hello Junio,
> Hello Felipe,
>
> Actually the issue happened while using Felipe's branch with his
> git-remote-hg.  Everything was going fine until I (or did it run
> automatically, I dont remember) ran git gc that pruned unreachable
> objects. Of course some of the branch I had pushed to the hg remote
> had been changed (most likely rebased).  References no longer exists
> in the repository (cleaned by gc), but the reference still exists in
> mark file, as it was exported earlier.  Thus the failure when git
> fast-export reads the mark file.

You described that part very well in your proposed log message and I
got it just fine.

> Then, is it safe ?
> Updating the last_idnum as I do in the patch doesn't work because
> if the reference is the last, the number is going to be overwriten
> in the next run.
> From git point of view, I guess it is fine. The file is fully read at
> the beginning of fast-export and fully written at the end.

I am not sure I follow the above, but anyway, I think the patch does
is safe because (1) future "fast-export" will not refer to these
pruned objects in its output (we have decided that these pruned
objects are not used anywhere in the history so nobody will refer to
them) and (2) we still need to increment the id number so that later
objects in the marks file get assigned the same id number as they
were assigned originally (otherwise we will not name these objects
consistently when we later talk about them).

And I wanted to see that kind of reasoning behind the patch in the
proposed log message, because other people will need to refer to it
when they read "git log" output to understand the change.

Thanks.

^ 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