Git development
 help / color / mirror / Atom feed
* New script to convert p4 repositories to git - git-p4c version 1.
From: John Chapman @ 2008-12-09 10:25 UTC (permalink / raw)
  To: Git Mailing List

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

I couldn't use git-p4 on my system because I kept running out of memory,
and I didn't like the workflow it imposed.
Also, it had various other issues with the repo I was trying to use,
mainly because it is not an ideal repository, however those are
(generally) the fault of the particular repo I was using, and not
git-p4. (Which is an excellent script by itself).

This script is severely crippled in that it doesn't (yet) allow one to
contribute changesets back to perforce, however it manages to read from
perforce with:
* No need to rebase.
* Mangling of file names. (Especially with regards to case sensitivity).
* Tagging of revisions with the perforce changesets.
* Ability to handle branches with spaces in the name.
* Ability to pretend that perforce doesn't exist. (That's the plan,
anyway).
* Be extremely memory efficient. It does NOT require as much memory as
does git-p4, even when the size of the change is large.
* Be easy to manually modify the repository, particularly if bad things
happen.

Unfortunately, not all of the above features may be reliable yet,
however I offer this script in order to obtain hopefully constructive
feedback so that I may improve the script and make it work very well.

Once I perfect this script, I plan to work on getting changes from git
back into perforce, which I have a few ideas as to how I might do it.
(None of which require rebasing).

It requires an OS that can efficiently utilise many open files and
pipes, and can run many processes. Such as Linux.  I seriously doubt it
can work on Windows.

It is called git-p4c, because 'git-p4' was taken, and I intended to
write it in C++.  I may still rewrite it in C++ if it is found
neccessary to use it on windows. (The Perforce C++ ABI will remove the
need to fork so many processes), but I won't be doing that before I
implement the write to perforce support.

Consider this to be experimental, not yet worthy of a version number.

Remember, I crave (constructive) feedback.

Thankyou.

[-- Attachment #2: git-p4c --]
[-- Type: text/x-python, Size: 17686 bytes --]

#!/usr/bin/env python

USAGE = r'''
git-p4c - written by John Chapman.

License:
    You are free to use this under the terms of the GPL License
    http://www.fsf.org/licensing/licenses/gpl.html

    I may change the license at any date in the future, unless
    I have substantial contributions, but regardless of what licence
    I choose, it will be an open source license.

    Probably it will become whatever license Git itself is under,
    just to make my life easier.

Example:
~/git-p4c/git-p4c \
--server=localhost:1666 \
--root=//depot \
--repo=/tmp/playground \
--user=arafangion \
--pass= \
--p4=/home/arafangion/perforce/p4 \
--max-changes=2 \
--branches='
trunk=//depot/(trunk)/(.*)
branches=//depot/branches/(.*?)/(.*)
'
'''

import datetime
import fcntl
import marshal
import os
import subprocess
import sys
import time
import sre

def main():
    opts = (
        '--server',
        '--user',
        '--pass',
        '--allow-case-changes',
        '--root',
        '--p4',
        '--branches',
        '--repo',
        '--initial',
        '--max-changes')

    config = git_config()

    # Now, override configuration if specified:
    for arg in sys.argv:
        for opt in opts:
            if arg.startswith(opt):
                config[opt[2:]] = arg.split('=', 1)[1]

    config = git_config(config)

    P4C = p4c_Connection(config)
    GIT = git_Connection(config)

    start = max(int(config['initial'])-1, GIT.latest())

    print 'Downloading Changesets...'
    c = 0
    t = time.time()
    for cs in P4C.changesets(start):
        if c != 0:
            print 'Processing:', cs.number, 'Avg: ', (time.time()-t)/float(c), ' at', datetime.datetime.today().ctime(),
        else:
            print 'Processing:', cs.number,
        g = GIT.commit(cs)
        if g is not None:
            for file in cs.files():
                if file.is_interesting():
                    sys.stdout.write('.')
                    g.add(file)
            sys.stdout.write('\n')
            g.commit()
        c += 1

        if c >= int(config['max-changes']):
            break

    print 'Fetch Complete!'

def git_config(conf=None):
    if conf is not None:
        if 'repo' in conf:
            try:
                os.mkdir(conf['repo'])
            except:
                pass
            os.chdir(conf['repo'])
        g = subprocess.Popen(('git', 'init'))
        g.wait()

        for key in conf:
            if '\n' in conf[key]:
                c = 1
                for line in conf[key].split('\n'):
                    line = line.strip()
                    if line=='':
                        continue
                    p = subprocess.Popen(('git', 'config', 'git-p4c.'+key+'-'+str(c), line))
                    p.wait()
                    c += 1
            else:
                p = subprocess.Popen(('git', 'config', 'git-p4c.'+key, conf[key]))
                p.wait()
    else:
        conf = {}

    p = subprocess.Popen(('git', 'config', '-l'), stdout=subprocess.PIPE)
    p.wait()
    conf = {}
    for line in p.stdout.readlines():
        line = line.strip()
        if line.startswith('git-p4c.'):
            key, value = line.split('=', 1)
            key = key[len('git-p4c.'):]
            if key.split('-')[0] == 'branches':
                if 'branches' not in conf:
                    conf['branches'] = []
                conf['branches'].append(value)
            else:
                conf[key] = value

    # Default Values:
    if 'initial' not in conf:
        conf['initial'] = '0'
    if 'max-changes' not in conf:
        conf['max-changes'] = '999999999'

    return conf

class git_Connection:
    def __init__(self, config):
        self._latest_mark = 1
        self._latest_changeset = 0
        self.config = config
        self._tags = {}
        cmd = ('git', 'fast-import')
        self._fast_import = subprocess.Popen(cmd,
            bufsize=0,
            stdin=subprocess.PIPE,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE)
        self._prev_p4changeset = None

        # Now, determine the current heads:
        g = subprocess.Popen(('git', 'tag'),
            stdout=subprocess.PIPE)
        heads = {}
        for line in g.stdout.readlines():
            line = line.strip()
            branch, number = unformat_tag(config, line)
            if branch not in heads:
                heads[branch] = [line, number]
            if heads[branch][1] < number:
                heads[branch][1] = number
        max = 0
        for head in heads:
            if max < heads[head][1]:
                max = heads[head][1]
            tag = 'refs/tags/'+heads[head][0]
            t = open('.git/'+tag, 'rb')
            committish = t.read().strip()
            self._record_tag(
                format_tag(
                    self.config, head, heads[head][1]),
                committish)
        self._latest_changeset = max
        self._heads = heads

    def _record_tag(self, tag, committish):
        self._tags[tag] = committish
    def tag_sha1(self, tag):
        return self._tags[tag]
    def heads(self):
        return self._heads
    def latest(self):
        'Returns the latest perforce changeset'
        return self._latest_changeset
    def commit(self, p4changeset):
        return git_Commit(self, p4changeset)
    def next_mark(self):
        'TODO: Ensure that the latest mark in the marks file is used as the starting point.'
        self._latest_mark += 1
        return self._latest_mark

class git_Commit:
    def __init__(self, connection, commit):
        self._con = connection
        self._commit = commit
        self._files = {}
    def add(self, p4file):
        if not self._files.has_key(p4file.branch()):
            self._files[p4file.branch()] = []
        self._files[p4file.branch()].append(p4file)

        if not p4file.action in ('delete', 'purge'):
            p4file.mark = self._con.next_mark()
            self._write('blob\nmark :%(mark)d\ndata %(size)d\n' % {
                'mark':p4file.mark,
                'size':p4file.size})

            data = 'foo'
            while data != '':
                try:
                    data = p4file.read(1024)
                    self._write(data)
                except:
                    time.sleep(0.1)
                    data = 'foo'
        p4file.close_files()
    def _write(self, s):
        self._con._fast_import.stdin.write(s)
    def commit(self):
        self._mark = self._con.next_mark()
        mark = self._mark
        for branch in self._files.keys():
            if branch in self._con.heads():
                from_tag = format_tag(self._con.config, branch, self._con.heads()[branch][1])
            else:
                from_tag = None
            self._con.heads()[branch] = [format_tag(self._con.config, branch, self._commit.number), self._commit.number]
            from_branch = self._files[branch][0].orig_branch()
            self._write(
'''commit %(ref)s
mark :%(mark)d
committer %(name)s <%(email)s> %(when)d +0000
data %(length)d
%(message)s
''' % {'ref':'refs/heads/'+branch,
                'mark':mark, 
                'name':self._commit.author(),
                'email':self._commit.email(),
                'when':self._commit.time(),
                'length':len(self._commit.commit_msg()),
                'message':self._commit.commit_msg()})
            if branch != from_branch:
                self._write(
                    'from %(from)s\n' %
                    {'from':'refs/heads/'+from_branch})
            elif from_tag is not None:
                self._write(
                    'from %(from)s\n' %
                    {'from':self._con.tag_sha1(from_tag)})

            for file in self._files[branch]:
                if file.action in ('add', 'edit', 'integrate', 'branch'):
                    self._c_add(file)
                elif file.action in ('delete', 'purge'):
                    self._c_delete(file)
                else:
                    print 'Unhandled action:', file

            tagname = format_tag(self._con.config, branch, self._commit.number)
            self._write(
'''tag %(tagname)s
from %(committish)s
tagger %(name)s <%(email)s> %(when)d +0000
data 0
''' % {
            'tagname':tagname,
            'committish':':'+str(mark),
            'name':self._commit.author(),
            'email':self._commit.email(),
            'when':self._commit.time()})
            self._con._record_tag(tagname, ':'+str(mark))
    def _c_add(self, file):
        self._write(
            '''M 100644 :%(mark)d %(path)s\n''' % {
                'path':file.name(),
                'mark':file.mark})
    def _c_delete(self, file):
        self._write('D %(path)s\n' % {'path':file.name()})

class p4c_Connection:
    def __init__(self, details):
        self._p4_exe = details['p4']
        self._p4port = details['server']
        self.config = details
        self._users = None
    def _p4(self, args):
        return subprocess.Popen(
            (self._p4_exe,)+args,
            bufsize=0,
            stdin=subprocess.PIPE,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE,
            env={'P4PORT':self._p4port,'P4PASSWD':self.config['pass'],'P4USER':self.config['user']},
            close_fds=True)
    def user(self, username):
        if self._users is None:
            p = self._p4(('-G', 'users'))
            try:
                self._users = {}
                while True:
                    d = marshal.load(p.stdout)
                    self._users[username] = {}
                    self._users[username]['email'] = d['Email']
                    self._users[username]['name'] = d['FullName']
            except EOFError, e:
                pass
        try:
            return self._users[username]
        except:
            return {'email':'Not A Current P4 User', 'name':username}

    def changesets(self, start):
        if self.config['root'][-1] != '/':
            self.config['root'] += '/'

        p = self._p4(('-G', 'changes', '-l', '-t', self.config['root']+'...'))

        try:
            self._changesets = []
            while True:
                d = marshal.load(p.stdout)
                self._changesets.append((int(d['change']), int(d['time'])))
        except EOFError, e:
            pass

        def s(x, y):
            if x[1] < y[1]: return -1
            if x[1] > y[1]: return 1
            return 0
        self._changesets.sort(s)
        for change, time in self._changesets:
            if change > start:
                yield self._Changeset(self, change, time)

    class _File:
        def __init__(self, connection, details):
            self._connection = connection
            self._details = details
            self.action = self._details['action']
            p = self._connection._p4(('-G', 'sizes', self.p4name()))
            try:
                if not self.action in ('delete', 'purge'):
                    self.size = marshal.load(p.stdout)
                    self.size = int(self.size['fileSize'])
            except Exception, e:
                self.size = 0
                self.action = 'purge'
            self._p = self._connection._p4(('print', '-q', self.p4name()))
            self.read = self._p.stdout.read

            oldflags = fcntl.fcntl(self._p.stdout, fcntl.F_GETFL)
            fcntl.fcntl(self._p.stdout, fcntl.F_SETFL, oldflags|os.O_NONBLOCK)

            try:
                self._branch_name, self._orig_branch, self._name = on_branch(self._connection.config, self.p4name())
            except:
                self._branch_name = None
                self._orig_branch = None
                self._name = None
        def p4name(self):
            return '//'+self._details['file']+'#'+self.rev()
        def branch(self):
            return self._branch_name
        def orig_branch(self):
            return self._orig_branch
        def name(self):
            return self._name
        def is_interesting(self):
            return self._branch_name is not None
        def rev(self):
            return self._details['rev']
        def tag(self):
            print self
            return self._details['tag']
        def __str__(self):
            return '\t'.join([key+' '+self._details[key] for key in self._details.keys()])
        def __del__(self):
            if self.read is not None:
                self.close_files() 
        def close_files(self):
            self.read = None
            self._p.stdout.close()
            self._p.stderr.close()
            self._p.stdin.close()
            self._p.wait()
            del self._p

    class _Changeset:
        def __init__(self, connection, number, time):
            self.number = number
            self._time = time
            self._connection = connection
            self._desc = {}
            self._files = {}

            p = self._connection._p4(('-G', 'describe',  str(self.number)))
            try:
                d = marshal.load(p.stdout)
                for key in d.keys():
                    if key[-1] in '0123456789':
                        'Is referring to a particular file.'
                        num = 0
                        name = ''
                        for c in key:
                            if c in '0123456789':
                                num *= 10
                                num += int(c)
                            else:
                                name += c
                        if not self._files.has_key(num):
                            self._files[num] = {}
                        'TODO: Determine which branch(es) this file belongs to.'
                        if name == 'depotFile':
                            self._files[num]['file'] = d[key][2:]
                        else:
                            self._files[num][name] = d[key]
                        self._files[num][name] = d[key]
                    else:
                        self._desc[key] = d[key]
            except EOFError, e:
                pass
        def __str__(self):
            return 'Changeset: %s Time: %s' % (self.number, self.time)
        def commit_msg(self):
            return self._desc['desc']
        def author(self):
            return self._connection.user(self._desc['client'])['name']+" '"+self._desc['client']+"'"
        def email(self):
            return self._connection.user(self._desc['client'])['email']
        def time(self):
            return self._time

        def files(self):
            for number in self._files.keys():
                yield self._connection._File(self._connection, self._files[number])


_seen = None
_tree = None
def generate_tree():
    '''Reads the current git repo and iterates over every branch, reading all files and
    directories, in order to ensure that file case does not ever change'''

    def get_branches():
        p = subprocess.Popen(('git', 'branch'), stdout=subprocess.PIPE)
        p.wait()
        for line in p.stdout.readlines():
            yield line.strip()
    def get_ls_tree(BranchOrSha1, dir=''):
        '''This function is very recursive,
        it returns ALL the trees (ie, the directories).'''
        p = subprocess.Popen(('git', 'ls-tree', BranchOrSha1), stdout=subprocess.PIPE)
        p.wait()
        for line in p.stdout.readlines():
            items = [item.strip() for item in line.strip().split(' ', 2)]
            last = items[-1]
            del items[-1]
            split_items = last.split('\t', 1)
            items.append(split_items[0])
            items.append(dir+split_items[1]+'/')

            if items[1] == 'tree':
                yield items
                for item in get_ls_tree(items[2], items[-1]):
                    yield item

    global _tree
    _tree = {}

    for branch in get_branches():
        for items in get_ls_tree(branch):
            _tree[items[-1][:-1].lower()] = items[-1][:-1].split('/')[-1]

def mangle_case(config, file):
    components = file.split('/')
    for i in range(len(components)-1):
        part = '/'.join(components[:i+1]).lower()
        if part in _tree:
            components[i] = _tree[part]
        else:
            _tree['/'.join(components[:i+1])] = components[i]

    file = '/'.join(components)
    return file

def on_branch(config, p4_filename):
    # TODO: Need to change this so that:
    # * If desired, prevent changes in case, either by simply preventing changes in case,
    #   or also by using an 'authoritative' perforce changeset.
    # * Stop using the stupid global for _seen, and consult the repo.
    #   (Currently it's worse than nothing, because
    #   it stuffs up the parent commits.)
    global _seen

    if _tree is None:
        generate_tree()

    if _seen is None:
        _seen = {}
        p = subprocess.Popen(('git', 'branch'), stdout=subprocess.PIPE)
        for line in p.stdout.readlines():
            _seen[line.strip()] = None
        p.wait()

    first=None
    for b in config['branches']:
        b, p = b.split('=', 1)

        if first is None:
            first = b
        m = sre.match('^'+p+'\#.*$', p4_filename)
        if m:
            branch, file = m.groups()
            if branch in _seen:
                return branch, branch, mangle_case(config, file)
            else:
                _seen[branch] = None
                return branch, first, mangle_case(config, file)

def format_tag(config, branch, number):
    return branch+'/'+str(number)

def unformat_tag(config, tag):
    branch, number = tag.rsplit('/', 1)
    number = int(number)

    return branch, number

if __name__ == '__main__':
    main()


^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Lars Hjemli @ 2008-12-09 10:57 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228819557.18611.80.camel@starfruit.local>

On Tue, Dec 9, 2008 at 11:45, R. Tyler Ballance <tyler@slide.com> wrote:
> Really hate to take this much bandwidth up on the mailing list over such
> a silly problem, but after spending a week trying to /talk/ and educate
> some folks, I feel drastic measures need to be taken ;)

A possible solution could be the "Integration manager workflow" described here:

  http://whygitisbetterthanx.com/#any-workflow

But it has the potential of confusing your co-devs ;-)

--
larsh

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Jeff King @ 2008-12-09 10:57 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: Nanako Shiraishi, git
In-Reply-To: <1228819087.18611.73.camel@starfruit.local>

On Tue, Dec 09, 2008 at 02:38:07AM -0800, R. Tyler Ballance wrote:

> At this point, QA is involved and what can happen is that QA realizes
> that this code is *not* stable and *never* should have been brought into
> the stable branch.
> 
> Now we have two options "block" the stable branch until LazyDeveloper
> makes the appropriate changes to stabilize the branch again *OR* back
> out LazyDeveloper's changes (A, B, C, D) and beat them up in the
> alleyway :)

It sounds like the problem is that LazyDeveloper has the authority to
push to the stable branch that everyone else pulls from, but can't be
trusted with that authority (because he is pushing bad work).

Maybe you would do better to invert your workflow:

  1. LazyDeveloper does some work on the 'foo' branch locally. Either
     his work repo is accessible to everyone, or he pushes it to a
     personal public repo (or a personal namespace within a shared
     repo).

  2. LazyDeveloper tells QA "check out foo, which should be ready for
     integration."

  3. QA pulls LazyDeveloper's foo. If it is OK, they merge and push to
     the official "stable" branch. If it isn't, they reject and
     LazyDeveloper fixes and goes back to step 2. LazyDeveloper is free
     to reset, rewind, or rebase as appropriate, since nobody but QA has
     ever even looked at this branch (and once they reached the "reject"
     conclusion, they don't care anymore).

So everyone builds off of the official "stable" branch, which by
definition is stuff that has passed through QA.

> Given the nature of our work, we have a stable branch per-team, and one
> funneling stable branch for the entire company (master), that branch
> being used to push the live web site with. 

And you could of course have per-team QA if you wanted to organize it
that way.

> The second option is why I want to force --no-ff on *all* pulls if
> possible. With --no-ff we can simply `git revert -sn <hash> -m 1 && git
> commit -a` in order to back out A, B, C, D. With a true fast-forward,
> we've had to use git-rev-list(1) trickery and some bash scriptery to
> properly revert a series of commits from a given time frame from a given
> developer.

There isn't good support for multiple reverts, but you can do the moral
equivalent with a big patch (note that revert can actually be more
clever about resolving the three way merge, but if you are close to the
tip, you shouldn't find any conflicts):

  git diff HEAD last-good-commit | git apply

If they are the tip commits, then you can always just make a new commit
with the pre-breakage state. This is sort of a mix of "git reset" and
"git revert" in that it throws away changes, but not history.

I don't think there is good porcelain support for this, but you can do:

  GIT_INDEX_FILE=index.tmp; export GIT_INDEX_FILE
  git read-tree last-good-commit
  git commit -m 'revert crappy commits'

-Peff

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: R. Tyler Ballance @ 2008-12-09 10:45 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: git
In-Reply-To: <8c5c35580812090231u28076844nf5a9225349c20801@mail.gmail.com>

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

On Tue, 2008-12-09 at 11:31 +0100, Lars Hjemli wrote:
> On Tue, Dec 9, 2008 at 11:12, R. Tyler Ballance <tyler@slide.com> wrote:
> > On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
> >> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> >> > Is there a header macro I can define or a config option I could define
> >> > to make --no-ff on `git pull` implicit instead of explicit?
> >>
> >> Try this:
> >> $ git config branch.stable.mergeoptions "--no-ff"
> >
> > I recall stumbling across this a while ago looking at the git-config(1)
> > man page, but this isn't /quite/ what we need.
> >
> > I'm talking about forcing for *every* pull, it's a safe assumption to
> > make that we want a merge commit every time somebody fast-forwards a
> > branch.
> 
> $ git config alias.xpull "pull --no-ff" ?

Interesting, I might have to try that out (wasn't aware of `git config
alias.<alias>`)

> 
> But are you sure you never want a fast-forward on _any_ branch? I use
> --no-ff unconditionally on the master and stable branches as $dayjob,
> to make sure that the merging of feature/bugfix-branches are
> explicitly noted in history, but I almost never use it on other
> branches.

I understand this, it's a funny situation. When we were evaluating Git
my team *never* had these issues because we all kept our trees in good
condition such that we never accidentally merged down to a stable
branch, but we also almost always generated merge commits because of the
variety of changes that would be going into stable at any given time.

I agree that I wouldn't want/need to use it on WIP branches or purely
local branches for development, so if I were able to restrict --no-ff to
only be forced on tracked branches I would be happy enough :)

Really hate to take this much bandwidth up on the mailing list over such
a silly problem, but after spending a week trying to /talk/ and educate
some folks, I feel drastic measures need to be taken ;)

Cheers 
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: R. Tyler Ballance @ 2008-12-09 10:38 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: git
In-Reply-To: <20081209191704.6117@nanako3.lavabit.com>

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

On Tue, 2008-12-09 at 19:17 +0900, Nanako Shiraishi wrote:
> Quoting "R. Tyler Ballance" <tyler@slide.com>:
> 
> > The most common use-case involves a user merging a project branch into a
> > stabilization branch (`git checkout stable && git pull . project`) in
> > such a way that no merge commit is generated. Of course, without
> > thinking they'll push these changes up to the centralized repository.
> > Not 15 minutes later they realize "ruh roh! I didn't want to do that"
> 
> Why does the user not want to fast-forward, if the merge she wants to do is actually a fast-forward?

I agree with you, this is more about preventing coworkers who are too
lazy to understand the entirety of what they're doing from hurting the
workflow of "the rest of us". It's a technically solution to a people
problem (I understand technology far more than people ;))

Consider the following scenarion:
  % git checkout -b project
  % <work>
  % git commit -am "A"
  % <work>
  % git commit -am "B"
  % <work>
  % git commit -am "C"
  % <work>
  % git commit -am "D"
  % git checkout stable
  % git pull . project
  % <fast-forward>
  % git push origin stable

At this point, QA is involved and what can happen is that QA realizes
that this code is *not* stable and *never* should have been brought into
the stable branch.

Now we have two options "block" the stable branch until LazyDeveloper
makes the appropriate changes to stabilize the branch again *OR* back
out LazyDeveloper's changes (A, B, C, D) and beat them up in the
alleyway :)

Given the nature of our work, we have a stable branch per-team, and one
funneling stable branch for the entire company (master), that branch
being used to push the live web site with. 

The first option (block) is not feasible as it will block the 40+ other
developers from pushing code until LazyDeveloper sufficiently gets their
crap together.

The second option is why I want to force --no-ff on *all* pulls if
possible. With --no-ff we can simply `git revert -sn <hash> -m 1 && git
commit -a` in order to back out A, B, C, D. With a true fast-forward,
we've had to use git-rev-list(1) trickery and some bash scriptery to
properly revert a series of commits from a given time frame from a given
developer.


> If you forbid fast-forward merges, when they merge their successful
> experiment back to the original topic, it will leave an unwanted merge
> in the history.

I'm less concerned at this point, the company switched entirely to Git
two weeks ago, with the history containing possible unwanted merges. I'm
more concerned however with LazyDeveloper inadvertently polluting stable
branches as LazyDeveloper does not yet fully grasp the concepts that Git
offers

> 
> In other words, I do not think --no-ff is a right solution for the problem you are trying to solve.  Perhaps you would need a hook that prevents a merge from certain direction from taking place instead?

If you do have a better solution to this problem (I dislike git push -f
origin[1]) I'm all ears, I'm more concerned with the end result at this
point ;)

Cheers


[1] We've stressed with our developers as much as possible that the
"origin" repository is to remain" pristine", that every action should be
"auditable" insofar that if you rollback a change, we want to see a
Revert commit, merges should create merge commits to where we can replay
or unwind the revision history correctly at any point in time or slice
of time. I *really* don't want "origin" to "lose commits".
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Lars Hjemli @ 2008-12-09 10:31 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228817565.18611.54.camel@starfruit.local>

On Tue, Dec 9, 2008 at 11:12, R. Tyler Ballance <tyler@slide.com> wrote:
> On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
>> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
>> > Is there a header macro I can define or a config option I could define
>> > to make --no-ff on `git pull` implicit instead of explicit?
>>
>> Try this:
>> $ git config branch.stable.mergeoptions "--no-ff"
>
> I recall stumbling across this a while ago looking at the git-config(1)
> man page, but this isn't /quite/ what we need.
>
> I'm talking about forcing for *every* pull, it's a safe assumption to
> make that we want a merge commit every time somebody fast-forwards a
> branch.

$ git config alias.xpull "pull --no-ff" ?

But are you sure you never want a fast-forward on _any_ branch? I use
--no-ff unconditionally on the master and stable branches as $dayjob,
to make sure that the merging of feature/bugfix-branches are
explicitly noted in history, but I almost never use it on other
branches.

--
larsh

^ permalink raw reply

* Re: git-bpush: Pushing to a bundle
From: Santi Béjar @ 2008-12-09 10:21 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git list
In-Reply-To: <alpine.DEB.1.00.0812091100470.2916@eeepc-johanness>

First, thanks for the comments.

2008/12/9 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Tue, 9 Dec 2008, Santi Béjar wrote:
>
>> The basic idea is:
>>
>> - Easily create bundles with the current branch.
>> - Be able to push to defined bundles in remote.<remote>.url
>> - Only add new objects by default (do not lose objects)
>
> That is probably not what people need.  Usually, when bundles are sent
> around, you need _incremental_ bundles.

I do not find convenient strictly incremental bundles, because then
you (or the other people) needs to fetch every single bundle. What I
do is add new objects until the bundle is too big and then create a
bundle with a new base. This way you don't have to worry if the other
person has applied the last bundle or not.

>  IOW if you already have a bundle,
> you want to create a new bundle that contains everything that is new, _in
> addition_ to the existing bundle.

>> while [ $# != 0 ] ; do
>
> Heh, I did not realize just how _used_ I got to the conventions in Git's
> shell programming, until I thought "Should this not use 'test' instead
> of brackets?"

I don't have problems either way, I'll change to follow Git's conventions.

>
>> while [ $# != 0 ] ; do
>>     refs="$refs$LF$1" && shift
>> done
>
> That is equivalent to refs="$*", no?

Almost, IFS is set to line-feed so I needed to put $LF instead of spaces.

>
> Anyway, I found reading your shell script quite hard, because of excessive
> use of brackets and single line && chains (which lack proper error
> handling, BTW).

I've tried to catch errors, but maybe not enough.

Santi

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Nanako Shiraishi @ 2008-12-09 10:17 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228815240.18611.48.camel@starfruit.local>

Quoting "R. Tyler Ballance" <tyler@slide.com>:

> The most common use-case involves a user merging a project branch into a
> stabilization branch (`git checkout stable && git pull . project`) in
> such a way that no merge commit is generated. Of course, without
> thinking they'll push these changes up to the centralized repository.
> Not 15 minutes later they realize "ruh roh! I didn't want to do that"

Why does the user not want to fast-forward, if the merge she wants to do is actually a fast-forward?

If you mean that the user merged branches in a wrong direction, how does it help her avoid such a mistake to unconditionally forbid fast-forward merges?  Doesn't people often do:

 Start on a topic branch, have a potentially bright idea...
 % git checkout -b experiment
 Hack on experiment branch.
 Happy because it indeed was an excellent idea.
 % git checkout topic
 % git pull . experiment
 % git branch -d experiement

If you forbid fast-forward merges, when they merge their successful experiment back to the original topic, it will leave an unwanted merge in the history.

In other words, I do not think --no-ff is a right solution for the problem you are trying to solve.  Perhaps you would need a hook that prevents a merge from certain direction from taking place instead?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: R. Tyler Ballance @ 2008-12-09 10:12 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: git
In-Reply-To: <8c5c35580812090149lc6dd79cj60a9d23c18089557@mail.gmail.com>

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

On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> > Is there a header macro I can define or a config option I could define
> > to make --no-ff on `git pull` implicit instead of explicit?
> 
> Try this:
> $ git config branch.stable.mergeoptions "--no-ff"

I recall stumbling across this a while ago looking at the git-config(1)
man page, but this isn't /quite/ what we need.

I'm talking about forcing for *every* pull, it's a safe assumption to
make that we want a merge commit every time somebody fast-forwards a
branch. 

The only way I could think to make use of branch.<name>.mergeoptions
would be to automagically set it up in a "pre-merge" hook, but alas
post-merge exists but not pre-merge.

I could certainly patch to support a pre-merge, but that seems like the
longest possible route to my desired destination ;)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: git-bpush: Pushing to a bundle
From: Johannes Schindelin @ 2008-12-09 10:07 UTC (permalink / raw)
  To: Santi Béjar; +Cc: git list
In-Reply-To: <adf1fd3d0812090149m158fcb9as15bacce58c61a1a3@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 963 bytes --]

Hi,

On Tue, 9 Dec 2008, Santi Béjar wrote:

> The basic idea is:
> 
> - Easily create bundles with the current branch.
> - Be able to push to defined bundles in remote.<remote>.url
> - Only add new objects by default (do not lose objects)

That is probably not what people need.  Usually, when bundles are sent 
around, you need _incremental_ bundles.  IOW if you already have a bundle, 
you want to create a new bundle that contains everything that is new, _in 
addition_ to the existing bundle.

> while [ $# != 0 ] ; do

Heh, I did not realize just how _used_ I got to the conventions in Git's 
shell programming, until I thought "Should this not use 'test' instead 
of brackets?"

> while [ $# != 0 ] ; do
>     refs="$refs$LF$1" && shift
> done

That is equivalent to refs="$*", no?

Anyway, I found reading your shell script quite hard, because of excessive 
use of brackets and single line && chains (which lack proper error 
handling, BTW).

Ciao,
Dscho

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Johannes Sixt @ 2008-12-09 10:00 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228815240.18611.48.camel@starfruit.local>

R. Tyler Ballance schrieb:
> The most common use-case involves a user merging a project branch into a
> stabilization branch (`git checkout stable && git pull . project`) in
> such a way that no merge commit is generated. Of course, without
> thinking they'll push these changes up to the centralized repository.
> Not 15 minutes later they realize "ruh roh! I didn't want to do that"
> and become very frustrated that they have to resort to asking for help
> or hand-reverting N number of commits. 

Is the problem

 * that there is no merge commit, or

 * that you have to undo N commits instead of just one?

The latter is probably helped by

   $ git reset --hard ORIG_HEAD && git push -f origin

-- Hannes

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Lars Hjemli @ 2008-12-09  9:49 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228815240.18611.48.camel@starfruit.local>

On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> Is there a header macro I can define or a config option I could define
> to make --no-ff on `git pull` implicit instead of explicit?

Try this:
$ git config branch.stable.mergeoptions "--no-ff"

--
lh

^ permalink raw reply

* git-bpush: Pushing to a bundle
From: Santi Béjar @ 2008-12-09  9:49 UTC (permalink / raw)
  To: git list

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

Hi *,

  I've made a script to push to a bundle that tries to behave as the
normal push.

  It has some limitations, but for the normal cases it works fine.

The basic idea is:

- Easily create bundles with the current branch.
- Be able to push to defined bundles in remote.<remote>.url
- Only add new objects by default (do not lose objects)
- Reuse existing bundles (keep the basis and the branches)
- Check that the branches fast-forward
- Keep the pushed branches in refs/remotes/<remote>/*

But it also has some limitations:

- Do not allow refspec (git-bundle do not support them), only branch/tags names.
- Push all branches or none (consequence of the above)
- ...

Hope this helps,
Santi

The scripts follows, but also attatched.
#!/bin/sh

OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
git bpush [options] [<remote> [<refs>...]]
--
f,force   force updates
full      create a full bundle
v         be verbose
"
SUBDIRECTORY_OK=Yes
. git-sh-setup
. git-parse-remote

cd_to_toplevel

LF='
'
IFS="$LF"

bases=
bbases=
changed=
force=
nonff=
remote=
refs=
while [ $# != 0 ] ; do
    case "$1" in
	-v) verbose=t;;
	--full) full=t;;
	-f|--force) force=t;;
	--) shift; break;;
	*) usage;;
    esac
    shift
done

[ -n "$1" ] && remote=$1 && shift
while [ $# != 0 ] ; do
    refs="$refs$LF$1" && shift
done

[ -z "$remote" ] && remote=$(get_default_remote)
remoteurl=$(git config remote.${remote}.url)
[ -z "$remoteurl" ] && remoteurl=$remote
[ -d "$remoteurl" ] && die "$remoteurl is a directory"

# Default bases in bundle.base
# Default {refs,base} can be specified in remote.<remote>.{push,bundlebase}
if [ "$remote" != "$remoteurl" ] ; then
    [ -z "$refs" ] &&
    refs=$(git config --get-all remote.${remote}.push)
    bases=$(git config --get-all remote.${remote}.bundlebase ||
	git config --get-all bundle.base)
else
    bases=$(git config --get-all bundle.base)
fi

# git rev-parse --symbolic-full-name resolves symlinks
# Keep at least HEAD
head=
for ref in $refs ; do
    [ "$ref" = HEAD ] && head=t && break
done

[ -n "$bases" ] && bases=$(git rev-parse --revs-only $bases | sort -u)
# Full symbolic refs to be uniq
[ -n "$refs" ] && \
    refs=$(git-rev-parse --symbolic-full-name --revs-only $refs | sort -u) && \
    [ -n "$head" ] && refs="HEAD$LF$refs"

if [ -e "$remoteurl" ] ; then
    # Find the bundle's bases
    refs="$refs$LF$(git bundle list-heads $remoteurl | cut -d " " -f 2)"
    requires=
    for line in $(git bundle verify "$remoteurl" 2>/dev/null) ; do
	case "$line" in "The bundle requires"*) requires=t && continue; esac
	[ -z "$requires" ] && continue
	bbase=$(echo $line | cut -d " " -f 1)
	[ -z "$bbases" ] && bbases=$bbase && continue
	bbases="$bbases$LF$bbase"
    done
    bbases=$(echo "$bbases" | sort -u)
    [ -z "$bases" ] && bases="$bbases" || bases="$bases$LF$bbases"
elif [ -z "$refs" ] ; then
    # Push current branch
    refs="HEAD$LF$(git symbolic-ref -q HEAD)"
fi

[ -z "$refs" ] && die "No refs to push"

refs=$(echo "$refs" | sort -u)

for ref in $bases $refs ; do
    [ "$(git cat-file -t $ref^{})" != commit ] && \
	die "$(basename $0): $ref is not a commit"
done

header="To $remoteurl"
[ -n "$verbose" ] && echo "Pushing to $remoteurl" && echo $header && header=

# Find what is/is not a fast-forward, up to date or new
# As "git bundle" does not support refspecs we must push all matching branches
for ref in $refs ; do
    case $ref in
	refs/tags/*) type=tags; newtext="new tag";;
	refs/heads/*|HEAD) type=heads ; newtext="new branch" ;;
    esac
    newhash=$(git rev-parse $ref)
    newshort=$(git rev-parse --short $ref)
    bshort=$(echo $ref | sed -e "s|^refs/$type/||")
    if [ -e "$remoteurl" ] ; then
	bheads="$(git bundle list-heads $remoteurl)"
	for bhead in $bheads ; do
	    bhash=$(echo $bhead | cut -d " " -f 1)
	    bref=$(echo $bhead | cut -d " " -f 2)
	    [ "$bref" != "$ref" ] && continue
	    oldshort=$(git-rev-parse --short $bhash)
	    case $type in
		tags)
		    base=$newhash;;
		heads)
		    base=$(git merge-base $bref $bhash);;
	    esac
	    if [ "$base" != $bhash ] ; then
		[ -n "$header" ] && echo $header && header=
		if [ -z "$force" ] ; then
		    nonff=t
		    echo " ! [rejected] $bshort -> $bshort (non-fast forward)"
		else
		    changed=t
		    echo " + $oldshort...$newshort $bshort -> $bshort (forced update)"
		fi
		continue 2
	    fi
	    if [ "$newhash" != "$bhash" ] ; then
		changed=t
		[ -n "$header" ] && echo $header && header=
		echo "   $oldshort..$newshort $bshort -> $bshort"
	    elif [ -n "$verbose" ] ; then
		[ -n "$header" ] && echo $header && header=
		echo " = [up to date] $bshort -> $bshort"
	    fi
	    continue 2
	done
    fi
    [ -n "$header" ] && echo $header && header=
    echo " * [$newtext] $bshort -> $bshort"
    changed=t
done

[ -n "$full" ] && bases= && [ -n "$bbases" ] && changed=t

[ -n "$nonff" ] && die "error: failed to push some refs to $remoteurl"
[ -z "$changed" ] && die "Everything up-to-date"
[ -n "$bases" ] && bases="--not$LF$bases"

git bundle create $remoteurl $refs $bases
[ "$remote" != "$remoteurl" ] && git fetch -q "$remote"
exit 0

[-- Attachment #2: git-bpush --]
[-- Type: application/octet-stream, Size: 4351 bytes --]

#!/bin/sh

OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
git bpush [options] [<remote> [<refs>...]]
--
f,force   force updates
full      create a full bundle
v         be verbose
"
SUBDIRECTORY_OK=Yes
. git-sh-setup
. git-parse-remote

cd_to_toplevel

LF='
'
IFS="$LF"

bases=
bbases=
changed=
force=
nonff=
remote=
refs=
while [ $# != 0 ] ; do
    case "$1" in
	-v) verbose=t;;
	--full) full=t;;
	-f|--force) force=t;;
	--) shift; break;;
	*) usage;;
    esac
    shift
done

[ -n "$1" ] && remote=$1 && shift
while [ $# != 0 ] ; do
    refs="$refs$LF$1" && shift
done

[ -z "$remote" ] && remote=$(get_default_remote)
remoteurl=$(git config remote.${remote}.url)
[ -z "$remoteurl" ] && remoteurl=$remote
[ -d "$remoteurl" ] && die "$remoteurl is a directory"

# Default bases in bundle.base
# Default {refs,base} can be specified in remote.<remote>.{push,bundlebase}
if [ "$remote" != "$remoteurl" ] ; then
    [ -z "$refs" ] &&
    refs=$(git config --get-all remote.${remote}.push)
    bases=$(git config --get-all remote.${remote}.bundlebase ||
	git config --get-all bundle.base)
else
    bases=$(git config --get-all bundle.base)
fi

# git rev-parse --symbolic-full-name resolves symlinks
# Keep at least HEAD
head=
for ref in $refs ; do
    [ "$ref" = HEAD ] && head=t && break
done

[ -n "$bases" ] && bases=$(git rev-parse --revs-only $bases | sort -u)
# Full symbolic refs to be uniq
[ -n "$refs" ] && \
    refs=$(git-rev-parse --symbolic-full-name --revs-only $refs | sort -u) && \
    [ -n "$head" ] && refs="HEAD$LF$refs"

if [ -e "$remoteurl" ] ; then
    # Find the bundle's bases
    refs="$refs$LF$(git bundle list-heads $remoteurl | cut -d " " -f 2)"
    requires=
    for line in $(git bundle verify "$remoteurl" 2>/dev/null) ; do
	case "$line" in "The bundle requires"*) requires=t && continue; esac
	[ -z "$requires" ] && continue
	bbase=$(echo $line | cut -d " " -f 1)
	[ -z "$bbases" ] && bbases=$bbase && continue
	bbases="$bbases$LF$bbase"
    done
    bbases=$(echo "$bbases" | sort -u)
    [ -z "$bases" ] && bases="$bbases" || bases="$bases$LF$bbases"
elif [ -z "$refs" ] ; then
    # Push current branch
    refs="HEAD$LF$(git symbolic-ref -q HEAD)"
fi

[ -z "$refs" ] && die "No refs to push"

refs=$(echo "$refs" | sort -u)

for ref in $bases $refs ; do
    [ "$(git cat-file -t $ref^{})" != commit ] && \
	die "$(basename $0): $ref is not a commit"
done

header="To $remoteurl"
[ -n "$verbose" ] && echo "Pushing to $remoteurl" && echo $header && header=

# Find what is/is not a fast-forward, up to date or new
# As "git bundle" does not support refspecs we must push all matching branches
for ref in $refs ; do
    case $ref in
	refs/tags/*) type=tags; newtext="new tag";;
	refs/heads/*|HEAD) type=heads ; newtext="new branch" ;;
    esac
    newhash=$(git rev-parse $ref)
    newshort=$(git rev-parse --short $ref)
    bshort=$(echo $ref | sed -e "s|^refs/$type/||")
    if [ -e "$remoteurl" ] ; then
	bheads="$(git bundle list-heads $remoteurl)"
	for bhead in $bheads ; do
	    bhash=$(echo $bhead | cut -d " " -f 1)
	    bref=$(echo $bhead | cut -d " " -f 2)
	    [ "$bref" != "$ref" ] && continue
	    oldshort=$(git-rev-parse --short $bhash)
	    case $type in
		tags)
		    base=$newhash;;
		heads)
		    base=$(git merge-base $bref $bhash);;
	    esac
	    if [ "$base" != $bhash ] ; then
		[ -n "$header" ] && echo $header && header=
		if [ -z "$force" ] ; then
		    nonff=t
		    echo " ! [rejected] $bshort -> $bshort (non-fast forward)"
		else
		    changed=t
		    echo " + $oldshort...$newshort $bshort -> $bshort (forced update)"
		fi
		continue 2
	    fi
	    if [ "$newhash" != "$bhash" ] ; then
		changed=t
		[ -n "$header" ] && echo $header && header=
		echo "   $oldshort..$newshort $bshort -> $bshort"
	    elif [ -n "$verbose" ] ; then
		[ -n "$header" ] && echo $header && header=
		echo " = [up to date] $bshort -> $bshort"
	    fi
	    continue 2
	done
    fi
    [ -n "$header" ] && echo $header && header=
    echo " * [$newtext] $bshort -> $bshort"
    changed=t
done

[ -n "$full" ] && bases= && [ -n "$bbases" ] && changed=t

[ -n "$nonff" ] && die "error: failed to push some refs to $remoteurl"
[ -z "$changed" ] && die "Everything up-to-date"
[ -n "$bases" ] && bases="--not$LF$bases"

git bundle create $remoteurl $refs $bases
[ "$remote" != "$remoteurl" ] && git fetch -q "$remote"
exit 0

^ permalink raw reply

* Re: Forcing --no-ff on pull
From: Jakub Narebski @ 2008-12-09  9:46 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1228815240.18611.48.camel@starfruit.local>

"R. Tyler Ballance" <tyler@slide.com> writes:

> Is there a header macro I can define or a config option I could define
> to make --no-ff on `git pull` implicit instead of explicit? Making sure
> we are always generating merge commits as a "just-in-case" safe guard
> about merge-happy developers who think after hitting enter? :)

branch.<name>.mergeoptions ?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: R. Tyler Ballance @ 2008-12-09  9:02 UTC (permalink / raw)
  To: Jan Krüger; +Cc: Git ML
In-Reply-To: <20081209093627.77039a1f@perceptron>

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

On Tue, 2008-12-09 at 09:36 +0100, Jan Krüger wrote:
> For fixing a corrupted repository by using backup copies of individual
> files, allow write_sha1_file() to write loose files even if the object
> already exists in a pack file, but only if the existing entry is marked
> as corrupted.
> 
> Signed-off-by: Jan Krüger <jk@jk.gs>
> ---
> 
> On IRC I talked to rtyler who had a corrupted pack file and plenty of
> object backups by way of cloned repositories. We decided to try
> extracting the corrupted objects from the other object database and
> injecting them into the broken repo as loose objects, but this failed
> because sha1_write_file() refuses to write loose objects that are
> already present in a pack file.

Figured I'd chime in here with some anecdotal evidence with the error
condition that I hit shortly after Jan sent the email.

        xdev3 (master-release)% git pull --no-ff . master
        From .
         * branch            master     -> FETCH_HEAD
        error: failed to read object
        befd9bc4d184b4383569909e4d245f3337c1f8ed at offset 1415784644
        from .git/objects/pack/pack-f7eb06e39f01b528c1d1a2c413ac51b31b8515aa.pack
        fatal: object befd9bc4d184b4383569909e4d245f3337c1f8ed is
        corrupted
        Merge with strategy recursive failed.
        xdev3 (master-release)%

I ran that command a couple of times to make sure it wasn't a fluke, I
repeated the error numerous times (without switching branches or pulling
from a remote). This pull was done with a slightly modified internal
version of v1.6.0.4
        xdev3 (master-release)% git --version
        git version 1.6.0.4-kb1
        xdev3 (master-release)
        

After consulting with Jan, I tried running the same command with a
modified version of v1.6.0.5 with Jan's patch
        xdev3 (master-release)% ~/basket/bin/git pull --no-ff . master
        From .
         * branch            master     -> FETCH_HEAD
        Merge made by recursive.
         ** TOP SECRET MERGES! ;) **
        
         13 files changed, 51 insertions(+), 21 deletions(-)
        xdev3 (master-release)%
        
        
Purely anecdotal as I'm not entirely clear what the hell is actually going on here :)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: is gitosis secure?
From: R. Tyler Ballance @ 2008-12-09  9:07 UTC (permalink / raw)
  To: Thomas Koch; +Cc: Git Mailing List, dabe
In-Reply-To: <200812090956.48613.thomas@koch.ro>

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

On Tue, 2008-12-09 at 09:56 +0100, Thomas Koch wrote:
> Sorry for the shameless subject, but I presented gitosis yesterday to
> our sysadmin and he wasn't much delighted to learn, that write access to
> repositories hosted with gitosis would need SSH access.

Accounts set up with keys for Gitosis are given restricted accounts
(from my understanding similar to how CVS or SVN operate over SSH
tunnels). 

The sysadmins here at Slide also had similar frustrations/concerns about
using Gitosis, but we were able to convince them that keys were a far
better solution than keyboard-interactive login sessions over HTTPS for
Subversion.

We're using gitosis with plenty of developers (coming up on 50) and
haven't had any issues with security (yet, crossed fingers). We even
have some accounts that are able to read but not write, i.e. they can
clone and pull, but not push back up to the central repository. YMMV.

> 
> So could you help me out in this discussion, whether to use or not to
> use gitosis? 
> Our admin would prefer to not open SSH at all outside our LAN, but
> developers would need to have write access also outside the office.

I recommend using VPN if the need to push/pull while outside of the
office (more fun solutions include SSH gateways that tunnel outside to
inside). Otherwise, why could they not simply commit locally, etc, and
then when they come into the office push/pull?

Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: is gitosis secure?
From: Sverre Rabbelier @ 2008-12-09  9:38 UTC (permalink / raw)
  To: Thomas Koch; +Cc: Git Mailing List, dabe
In-Reply-To: <200812090956.48613.thomas@koch.ro>

On Tue, Dec 9, 2008 at 09:56, Thomas Koch <thomas@koch.ro> wrote:
> Our admin would prefer to not open SSH at all outside our LAN, but
> developers would need to have write access also outside the office.

What safer to connect to the LAN than with SSH? What _would_ your
system admin be happy with using?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Forcing --no-ff on pull
From: R. Tyler Ballance @ 2008-12-09  9:34 UTC (permalink / raw)
  To: git

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

While I'm in the email writing mood tonight, I figured I'd ask this
question.

We've recently moved a giant tree with a number of developers over to
Git from Subversion. One of the biggest stumbling points we have right
now is the concept of a "fast-forward", insofar that it's "screwed" us a
couple times (see: people not RTFM'ing then crying that Git is broken
because they cannot RTFM ;))

The most common use-case involves a user merging a project branch into a
stabilization branch (`git checkout stable && git pull . project`) in
such a way that no merge commit is generated. Of course, without
thinking they'll push these changes up to the centralized repository.
Not 15 minutes later they realize "ruh roh! I didn't want to do that"
and become very frustrated that they have to resort to asking for help
or hand-reverting N number of commits. 

Is there a header macro I can define or a config option I could define
to make --no-ff on `git pull` implicit instead of explicit? Making sure
we are always generating merge commits as a "just-in-case" safe guard
about merge-happy developers who think after hitting enter? :)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Lars Hjemli @ 2008-12-09  9:23 UTC (permalink / raw)
  To: Fabian Franz; +Cc: git
In-Reply-To: <1228784261-18637-1-git-send-email-git@fabian-franz.de>

On Tue, Dec 9, 2008 at 01:57, Fabian Franz <git@fabian-franz.de> wrote:
> Technically the gitlink code was changed to read .git/HEAD.gitlink
> if it exists instead of the normal HEAD. If you add 0000* as sha1
> sum to .git/HEAD.gitlink the submodule code will always fetch HEAD.

This feels like the porcelain "fooling" the plumbing. How about
something like this instead:

diff --git a/read-cache.c b/read-cache.c
index 8579663..cfacea7 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -137,6 +137,8 @@ static int ce_compare_gitlink(struct cache_entry *ce)
         */
        if (resolve_gitlink_ref(ce->name, "HEAD", sha1) < 0)
                return 0;
+       if (is_null_sha1(ce->sha1))
+               return 0;
        return hashcmp(sha1, ce->sha1);
 }

This should make the plumbing happy no matter which commit is actually
checked out in the submodule (not actually tested...). Then,
cmd_update() can check if the requested sha1 is all '0' and
fetch+checkout latest HEAD (or some branch) without playing games with
.git/HEAD.gitlink. Finally, cmd_add() needs to update the index in the
containing repository with the magic '0*' sha1 if '--track' is
specifed. This can be achieved by replacing 'git add $path' with 'echo
$mode $sha1\t$path | git update-index --index-info'.

What do you think?

> @@ -327,10 +335,14 @@ cmd_update()
>                        say "Maybe you want to use 'update --init'?"
>                        continue
>                fi
> +               track=$(git config -f .gitmodules submodule."$name".track)

I'm pretty certain that we don't want to use info from .gitmodules in
cmd_update(). Instead, cmd_init() probably should move the info from
.gitmodules into .git/config and cmd_update() should check the latter.

Btw: cmd_status() probably also needs some modifications to handle
this special case.

--
larsh

^ permalink raw reply related

* is gitosis secure?
From: Thomas Koch @ 2008-12-09  8:56 UTC (permalink / raw)
  To: Git Mailing List, dabe

Sorry for the shameless subject, but I presented gitosis yesterday to
our sysadmin and he wasn't much delighted to learn, that write access to
repositories hosted with gitosis would need SSH access.

So could you help me out in this discussion, whether to use or not to
use gitosis? 
Our admin would prefer to not open SSH at all outside our LAN, but
developers would need to have write access also outside the office.

Best regards,
-- 
Thomas Koch, Software Developer
http://www.koch.ro

Young Media Concepts GmbH
Sonnenstr. 4
CH-8280 Kreuzlingen
Switzerland

Tel    +41 (0)71 / 508 24 86
Fax    +41 (0)71 / 560 53 89
Mobile +49 (0)170 / 753 89 16
Web    www.ymc.ch

^ permalink raw reply

* Re: How to clone git repository with git-svn meta-data included?
From: Michael J Gruber @ 2008-12-09  8:53 UTC (permalink / raw)
  To: Grzegorz Kossakowski; +Cc: Michael J Gruber, git
In-Reply-To: <493D66BB.3060907@tuffmail.com>

Grzegorz Kossakowski venit, vidit, dixit 08.12.2008 19:26:
> Michael J Gruber pisze:
>> Could it be as simple as a missing "cd cocoon" between git clone and git
>> svn rebase? No, you probably did that.
> 
> That would be too easy.
> 
>> But note that you did not follow Peter's instructions. The point is that
>> your clone creates "remotes/origin/trunk" whereas Peter's instructions
>> mirror the source, creating "remotes/trunk", which is what git svn needs
>> (unless you say "git svn init -s --prefix=origin/" or "git config
>> svn-remote.svn.fetch trunk:refs/trunk" etc.). The prefix solution should
>> be the best.
>>
>> Michael
>>
>> P.S.: Peter starts off a different layout (standard svn remotes, which
>> need special instructions to be cloned). Ordinary clone + git svn init
>> --prefix=origin/ should work fine for the cocoon layout.
> 
> This almost worked. Actually, Cocoon repository hosted on Jukka's server does not have local head
> named "trunk" so there is no remotes/origin/trunk created during cloning process.
> 
> I had to run:
> 
>   git update-ref refs/remotes/origin/trunk refs/heads/master

Uhm, I misread gitweb output... So, cocoon really has remotes/trunk, so
that Peter's original suggestion would work. If the cocoon git-svn clone
isn't going to change then Peter's way of doing it might be the best: it
ensures that future pulls from origin (cocoon) will update the correct
refs so that you can pull/fetch from the git-svn clone (fast) and then
git-svn rebase rather than git-svn rebasing directly, which would fetch
new commits from the svn repo first (slow).

I think all in all it shows that git-svns default location for branches
is not the best choice: they're not local, but they're no "proper"
remotes either (unless you use --prefix).

Michael

^ permalink raw reply

* [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: Jan Krüger @ 2008-12-09  8:36 UTC (permalink / raw)
  To: Git ML; +Cc: tyler

For fixing a corrupted repository by using backup copies of individual
files, allow write_sha1_file() to write loose files even if the object
already exists in a pack file, but only if the existing entry is marked
as corrupted.

Signed-off-by: Jan Krüger <jk@jk.gs>
---

On IRC I talked to rtyler who had a corrupted pack file and plenty of
object backups by way of cloned repositories. We decided to try
extracting the corrupted objects from the other object database and
injecting them into the broken repo as loose objects, but this failed
because sha1_write_file() refuses to write loose objects that are
already present in a pack file.

This patch expands the check to see if the pack entry has been marked
as corrupted and, if so, allows writing a loose object with the same
ID. Unfortunately, when Tyler tried a merge while using this patch,
something we didn't manage to track down happened and now git doesn't
consider the object corrupted anymore. I'm not sure enough that it
wasn't caused by the patch to submit this patch without hesitation.

Apart from that, I think the change is not all too great since it makes
write_sha1_file() walk the list of pack entries twice. That's a bit of
a waste.

So those are the reasons why I wanted a few opinions first. Another
reason is that there might be a way smarter method to fix this kind of
problem, in which case I'd love hearing about it for future reference.

 sha1_file.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 6c0e251..17085cc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2373,14 +2373,17 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
 	char hdr[32];
 	int hdrlen;
 
-	/* Normally if we have it in the pack then we do not bother writing
-	 * it out into .git/objects/??/?{38} file.
-	 */
 	write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
 	if (returnsha1)
 		hashcpy(returnsha1, sha1);
-	if (has_sha1_file(sha1))
-		return 0;
+	/* Normally if we have it in the pack then we do not bother writing
+	 * it out into .git/objects/??/?{38} file. We do, though, if there
+	 * is no chance that we have an uncorrupted version of the object.
+	 */
+	if (has_sha1_file(sha1)) {
+		if (has_loose_object(sha1) || !has_packed_and_bad(sha1))
+			return 0;
+	}
 	return write_loose_object(sha1, hdr, hdrlen, buf, len, 0);
 }
 
-- 
1.6.0.4.766.g6fc4a

^ permalink raw reply related

* [PATCH 2/2] diff: respect textconv in rewrite diffs
From: Jeff King @ 2008-12-09  8:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20081209081227.GA19626@coredump.intra.peff.net>

Currently we just skip rewrite diffs for binary files; this
patch makes an exception for files which will be textconv'd,
and actually performs the textconv before generating the
diff.

Conceptually, rewrite diffs should be in the exact same
format as the a non-rewrite diff, except that we refuse to
share any context. Thus it makes very little sense for "git
diff" to show a textconv'd diff, but for "git diff -B" to
show "Binary files differ".

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c                         |   48 ++++++++++++++++++++++++++++++----------
 t/t4031-diff-rewrite-binary.sh |   21 ++++++++++++++++-
 2 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index ea958a2..6dc19bc 100644
--- a/diff.c
+++ b/diff.c
@@ -229,6 +229,8 @@ static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
 			      struct diff_filespec *two,
+			      const char *textconv_one,
+			      const char *textconv_two,
 			      struct diff_options *o)
 {
 	int lc_a, lc_b;
@@ -241,6 +243,8 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *reset = diff_get_color(color_diff, DIFF_RESET);
 	static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
 	const char *a_prefix, *b_prefix;
+	const char *data_one, *data_two;
+	size_t size_one, size_two;
 
 	if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		a_prefix = o->b_prefix;
@@ -262,8 +266,27 @@ static void emit_rewrite_diff(const char *name_a,
 
 	diff_populate_filespec(one, 0);
 	diff_populate_filespec(two, 0);
-	lc_a = count_lines(one->data, one->size);
-	lc_b = count_lines(two->data, two->size);
+	if (textconv_one) {
+		data_one = run_textconv(textconv_one, one, &size_one);
+		if (!data_one)
+			die("unable to read files to diff");
+	}
+	else {
+		data_one = one->data;
+		size_one = one->size;
+	}
+	if (textconv_two) {
+		data_two = run_textconv(textconv_two, two, &size_two);
+		if (!data_two)
+			die("unable to read files to diff");
+	}
+	else {
+		data_two = two->data;
+		size_two = two->size;
+	}
+
+	lc_a = count_lines(data_one, size_one);
+	lc_b = count_lines(data_two, size_two);
 	fprintf(o->file,
 		"%s--- %s%s%s\n%s+++ %s%s%s\n%s@@ -",
 		metainfo, a_name.buf, name_a_tab, reset,
@@ -273,9 +296,9 @@ static void emit_rewrite_diff(const char *name_a,
 	print_line_count(o->file, lc_b);
 	fprintf(o->file, " @@%s\n", reset);
 	if (lc_a)
-		copy_file_with_prefix(o->file, '-', one->data, one->size, old, reset);
+		copy_file_with_prefix(o->file, '-', data_one, size_one, old, reset);
 	if (lc_b)
-		copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
+		copy_file_with_prefix(o->file, '+', data_two, size_two, new, reset);
 }
 
 static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
@@ -1334,6 +1357,11 @@ static void builtin_diff(const char *name_a,
 	const char *a_prefix, *b_prefix;
 	const char *textconv_one = NULL, *textconv_two = NULL;
 
+	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+		textconv_one = get_textconv(one);
+		textconv_two = get_textconv(two);
+	}
+
 	diff_set_mnemonic_prefix(o, "a/", "b/");
 	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		a_prefix = o->b_prefix;
@@ -1377,9 +1405,10 @@ static void builtin_diff(const char *name_a,
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
 		if (complete_rewrite &&
-		    !diff_filespec_is_binary(one) &&
-		    !diff_filespec_is_binary(two)) {
-			emit_rewrite_diff(name_a, name_b, one, two, o);
+		    (textconv_one || !diff_filespec_is_binary(one)) &&
+		    (textconv_two || !diff_filespec_is_binary(two))) {
+			emit_rewrite_diff(name_a, name_b, one, two,
+						textconv_one, textconv_two, o);
 			o->found_changes = 1;
 			goto free_ab_and_return;
 		}
@@ -1388,11 +1417,6 @@ static void builtin_diff(const char *name_a,
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
-	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
-		textconv_one = get_textconv(one);
-		textconv_two = get_textconv(two);
-	}
-
 	if (!DIFF_OPT_TST(o, TEXT) &&
 	    ( (diff_filespec_is_binary(one) && !textconv_one) ||
 	      (diff_filespec_is_binary(two) && !textconv_two) )) {
diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index 4b522f7..72e5bfd 100755
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -7,13 +7,15 @@ test_description='rewrite diff on binary file'
 # We must be large enough to meet the MINIMUM_BREAK_SIZE
 # requirement.
 make_file() {
+	# common first line to help identify rewrite versus regular diff
+	printf "=\n" >file
 	for i in 1 2 3 4 5 6 7 8 9 10; do
 		for j in 1 2 3 4 5 6 7 9 10; do
 			for k in 1 2 3 4 5; do
 				printf "$1\n"
 			done
 		done
-	done >file
+	done >>file
 }
 
 test_expect_success 'create binary file with changes' '
@@ -39,4 +41,21 @@ test_expect_success 'rewrite diff can show binary patch' '
 	grep "GIT binary patch" diff
 '
 
+cat >dump <<'EOF'
+#!/bin/sh
+perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1"
+EOF
+chmod +x dump
+test_expect_success 'setup textconv' '
+	echo file diff=foo >.gitattributes &&
+	git config diff.foo.textconv "$PWD"/dump
+'
+
+test_expect_success 'rewrite diff respects textconv' '
+	git diff -B >diff &&
+	grep "dissimilarity index" diff &&
+	grep "^-61" diff &&
+	grep "^-0" diff
+'
+
 test_done
-- 
1.6.1.rc2.1.g8f945.dirty

^ permalink raw reply related

* [PATCH 1/2] diff: fix handling of binary rewrite diffs
From: Jeff King @ 2008-12-09  8:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The current emit_rewrite_diff code always writes a text
patch without checking whether the content is binary. This
means that if you end up with a rewrite diff for a binary
file, you get lots of raw binary goo in your patch.

Instead, if we have binary files, then let's just skip
emit_rewrite_diff altogether. We will already have shown the
"dissimilarity index" line, so it is really about the diff
contents. If binary diffs are turned off, the "Binary files
a/file and b/file differ" message should be the same in
either case. If we do have binary patches turned on, there
isn't much point in making a less-efficient binary patch
that does a total rewrite; no human is going to read it, and
since binary patches don't apply with any fuzz anyway, the
result of application should be the same.

Signed-off-by: Jeff King <peff@peff.net>
---
I couldn't think of a good reason to want a different format of binary
patch for a rewrite versus a regular diff, but others may be able to.
In that case, you have to mimic the binary codepath somewhat in
emit_rewrite_diff; I started on it, but realized I was just copying the
code.

However, we should at least do something before 1.6.1; as it is, it
spews binary garbage.

I discovered this because one of my textconv'd files had a rewrite
(which apparently happens by changing a few lines in a word document :))
and gave me a bogus diff. However, the problem is not unique to
textconv; patch 1/2 handles binary files and 2/2 handles the textconv
case.

I didn't put the tests into the existing t4022-diff-rewrite because I
wanted to keep the textconv tests together with these tests, and I think
it makes sense to test features in isolation before testing them
together. IOW:

  t4022 - rewrite works
  t4030 - textconv works
  t4031 - rewrite AND textconv work together

 diff.c                         |    4 ++-
 t/t4031-diff-rewrite-binary.sh |   42 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletions(-)
 create mode 100755 t/t4031-diff-rewrite-binary.sh

diff --git a/diff.c b/diff.c
index f644947..ea958a2 100644
--- a/diff.c
+++ b/diff.c
@@ -1376,7 +1376,9 @@ static void builtin_diff(const char *name_a,
 		 */
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
-		if (complete_rewrite) {
+		if (complete_rewrite &&
+		    !diff_filespec_is_binary(one) &&
+		    !diff_filespec_is_binary(two)) {
 			emit_rewrite_diff(name_a, name_b, one, two, o);
 			o->found_changes = 1;
 			goto free_ab_and_return;
diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
new file mode 100755
index 0000000..4b522f7
--- /dev/null
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='rewrite diff on binary file'
+
+. ./test-lib.sh
+
+# We must be large enough to meet the MINIMUM_BREAK_SIZE
+# requirement.
+make_file() {
+	for i in 1 2 3 4 5 6 7 8 9 10; do
+		for j in 1 2 3 4 5 6 7 9 10; do
+			for k in 1 2 3 4 5; do
+				printf "$1\n"
+			done
+		done
+	done >file
+}
+
+test_expect_success 'create binary file with changes' '
+	make_file "\\0" &&
+	git add file &&
+	make_file "\\01"
+'
+
+test_expect_success 'vanilla diff is binary' '
+	git diff >diff &&
+	grep "Binary files a/file and b/file differ" diff
+'
+
+test_expect_success 'rewrite diff is binary' '
+	git diff -B >diff &&
+	grep "dissimilarity index" diff &&
+	grep "Binary files a/file and b/file differ" diff
+'
+
+test_expect_success 'rewrite diff can show binary patch' '
+	git diff -B --binary >diff &&
+	grep "dissimilarity index" diff &&
+	grep "GIT binary patch" diff
+'
+
+test_done
-- 
1.6.1.rc2.1.g8f945.dirty

^ permalink raw reply related

* Why doesn't "git log --cherry-pick" work with ranges?
From: Kristian Amlie @ 2008-12-09  7:52 UTC (permalink / raw)
  To: Git Mailing List

Is it because A..B internally translates to B ^A?

Is this something that would be easy to add?

--
Kristian Amlie

^ 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