crash in git_attrs.git_attribute function

A recent change in this function sometimes causes a crash when
pushing a commit. This happens in a situation such as the following:

The user has a repository that looks like this:

     <root>/
        subdir/
           .gitattributes
           sub_sub_dir/
              .gitattributes
              some_file

If the user creates a commit which modifies some_file, the git_attribute
function will be passed a list of files which includes:

    subdir/sub_sub_dir/some_file

It will process that file by first looking at subdir/sub_sub_dir,
check if there is a .gitattributes in it (yes), so create that
directory, add it to the dirs_with_changes "list" so we know
we've already processed that directory. So far, so good.

Loop again, this time with os.path.dirname('subdir/sub_sub_dir'),
thus with 'subdir'. Is that directory in dirs_with_changes? No.
Does it have a .gitattribute in it? Yes.  So, rince-repeat, which
means first creating the subdir. That leads to the crash, since
we already just created it as a side-effect of creating its subdir.

This patch fixes the issue by simply remembering when a directory
has been created for a given file in filename_list.  Once that's
done, we know that the next iterations on the dir_path loop are
necessarily for parent directories, which have therefore been
created, even if they are not listed in the dirs_with_changes
"list" yet.

Another possible fix, which would be safer, would be to just
check the filesystem directly. But this code can be a hot-spot
in terms of performance, so we'll try really hard to avoid it.

Fixes P601-011.
Related to P531-036.
This commit is contained in:
Joel Brobecker
2016-06-01 10:07:18 -07:00
parent 9a7944833e
commit 1d1d4e7b3a
5 changed files with 76 additions and 1 deletions

View File

@@ -129,13 +129,16 @@ def git_attribute(commit_rev, filename_list, attr_name):
for filename in filename_list:
assert not os.path.isabs(filename)
dir_path = filename
dir_created = False
while dir_path:
dir_path = os.path.dirname(dir_path)
if dir_path in dirs_with_changes:
continue
gitattributes_rel_file = os.path.join(dir_path, '.gitattributes')
if cached_file_exists(commit_rev, gitattributes_rel_file):
os.makedirs(os.path.join(tmp_checkout_dir, dir_path))
if not dir_created:
os.makedirs(os.path.join(tmp_checkout_dir, dir_path))
dir_created = True
git.show("%s:%s" % (commit_rev, gitattributes_rel_file),
_outfile=os.path.join(tmp_checkout_dir,
gitattributes_rel_file))

View File

@@ -0,0 +1,4 @@
[core]
repositoryformatversion = 0
filemode = true
bare = true

View File

@@ -0,0 +1,3 @@
[hooks]
from-domain = adacore.com
mailinglist = git-hooks-ci@example.com

View File

@@ -0,0 +1,65 @@
from support import *
class TestRun(TestCase):
def test_push_commit_on_master(self):
"""Try pushing one single-file commit on master.
"""
cd ('%s/repo' % TEST_DIR)
# Push master to the `origin' remote. The delta should be one
# commit with one file being modified.
p = Run('git push origin master'.split())
expected_out = """\
remote: DEBUG: Content-Type: text/plain; charset="us-ascii"
remote: MIME-Version: 1.0
remote: Content-Transfer-Encoding: 7bit
remote: From: Test Suite <testsuite@adacore.com>
remote: To: git-hooks-ci@example.com
remote: Bcc: file-ci@gnat.com
remote: Subject: [repo] Minor reformatting.
remote: X-Act-Checkin: repo
remote: X-Git-Author: Joel Brobecker <brobecker@adacore.com>
remote: X-Git-Refname: refs/heads/master
remote: X-Git-Oldrev: 937ea8ac48994bdef9928dc6e246bb1d3efb8acd
remote: X-Git-Newrev: 4b27b37bcd68460e2ec2ccd97a4e93660ed4cd16
remote:
remote: commit 4b27b37bcd68460e2ec2ccd97a4e93660ed4cd16
remote: Author: Joel Brobecker <brobecker@adacore.com>
remote: Date: Wed Jun 1 09:38:20 2016 -0700
remote:
remote: Minor reformatting.
remote:
remote: Diff:
remote: ---
remote: gms/hello.adb | 1 +
remote: sec/README | 3 +++
remote: 2 files changed, 4 insertions(+)
remote:
remote: diff --git a/gms/hello.adb b/gms/hello.adb
remote: index 9a6ff61..93db53d 100644
remote: --- a/gms/hello.adb
remote: +++ b/gms/hello.adb
remote: @@ -1,4 +1,5 @@
remote: with Ada.Text_IO; use Ada.Text_IO;
remote: +
remote: procedure Hello is
remote: begin
remote: Put_Line ("Hello World.");
remote: diff --git a/sec/README b/sec/README
remote: index a7e57ad..93f282d 100644
remote: --- a/sec/README
remote: +++ b/sec/README
remote: @@ -1 +1,4 @@
remote: +REPO's sec/ README:
remote: +-------------------
remote: +
remote: This directory contains interesting stuff.
To ../bare/repo.git
937ea8a..4b27b37 master -> master
"""
self.assertEqual(p.status, 0, p.image)
self.assertRunOutputEqual(p, expected_out)
if __name__ == '__main__':
runtests()