diff --git a/lib/git/config.py b/lib/git/config.py index e919838b4..8541dc0eb 100644 --- a/lib/git/config.py +++ b/lib/git/config.py @@ -271,9 +271,9 @@ def read(self): if not hasattr(file_object, "seek"): try: fp = open(file_object) + close_fp = True except IOError,e: continue - close_fp = True # END fp handling try: @@ -308,17 +308,21 @@ def write(self): :raise IOError: if this is a read-only writer instance or if we could not obtain a file lock""" self._assure_writable("write") - self._lock._obtain_lock() - fp = self._file_or_files close_fp = False + # we have a physical file on disk, so get a lock + if isinstance(fp, (basestring, file)): + self._lock._obtain_lock() + # END get lock for physical files + if not hasattr(fp, "seek"): fp = open(self._file_or_files, "w") close_fp = True else: fp.seek(0) + # END handle stream or file # WRITE DATA try: @@ -390,7 +394,7 @@ def get_value(self, section, option, default = None): return valuestr @needs_values - @set_dirty_and_flush_changes + @set_dirty_and_flush_changes def set_value(self, section, option, value): """Sets the given option in section to the given value. It will create the section if required, and will not throw as opposed to the default diff --git a/lib/git/objects/submodule.py b/lib/git/objects/submodule.py index b0fd0e35f..b9bcfc079 100644 --- a/lib/git/objects/submodule.py +++ b/lib/git/objects/submodule.py @@ -6,6 +6,13 @@ __all__ = ("Submodule", ) +class SubmoduleConfigParser(GitConfigParser): + """Catches calls to _write, and updates the .gitmodules blob in the index + with the new data, if we have written into a stream. Otherwise it will + add the local file to the index to make it correspond with the working tree.""" + _mutating_methods_ = tuple() + + class Submodule(base.IndexObject): """Implements access to a git submodule. They are special in that their sha represents a commit in the submodule's repository which is to be checked out @@ -20,14 +27,14 @@ class Submodule(base.IndexObject): # this is a bogus type for base class compatability type = 'submodule' - __slots__ = ('_root_tree', '_url', '_ref') + __slots__ = ('_parent_commit', '_url', '_ref') def _set_cache_(self, attr): if attr == 'size': raise ValueError("Submodules do not have a size as they do not refer to anything in this repository") - elif attr == '_root_tree': + elif attr == '_parent_commit': # set a default value, which is the root tree of the current head - self._root_tree = self.repo.tree() + self._parent_commit = self.repo.commit() elif attr in ('path', '_url', '_ref'): reader = self.config_reader() # default submodule values @@ -39,13 +46,26 @@ def _set_cache_(self, attr): super(Submodule, self)._set_cache_(attr) # END handle attribute name - def _fp_config(self): + def _sio_modules(self): """:return: Configuration file as StringIO - we only access it through the respective blob's data""" - return StringIO(self._root_tree[self.kModulesFile].datastream.read()) + sio = StringIO(self._parent_commit.tree[self.kModulesFile].datastream.read()) + sio.name = self.kModulesFile + return sio def _config_parser(self, read_only): """:return: Config Parser constrained to our submodule in read or write mode""" - parser = GitConfigParser(self._fp_config(), read_only = read_only) + parent_matches_head = self.repo.head.commit == self._parent_commit + if not self.repo.bare and parent_matches_head: + fp_module = self.kModulesFile + else: + fp_module = self._sio_modules() + # END handle non-bare working tree + + if not read_only and not parent_matches_head: + raise ValueError("Cannot write blobs of 'historical' submodule configurations") + # END handle writes of historical submodules + + parser = GitConfigParser(fp_module, read_only = read_only) return SectionConstraint(parser, 'submodule "%s"' % self.path) #{ Edit Interface @@ -61,21 +81,24 @@ def add(cls, repo, path, url, skip_init=False): :param skip_init: if True, the new repository will not be cloned to its location. :return: The newly created submodule instance""" - def set_root_tree(self, root_tree): - """Set this instance to use the given tree which is supposed to contain the - .gitmodules blob. - :param root_tree: Tree'ish reference pointing at the root_tree - :raise ValueError: if the root_tree didn't contain the .gitmodules blob.""" - tree = self.repo.tree(root_tree) - if self.kModulesFile not in tree: - raise ValueError("Tree %s did not contain the %s file" % (root_tree, self.kModulesFile)) + def set_parent_commit(self, commit): + """Set this instance to use the given commit whose tree is supposed to + contain the .gitmodules blob. + :param commit: Commit'ish reference pointing at the root_tree + :raise ValueError: if the commit's tree didn't contain the .gitmodules blob.""" + pcommit = self.repo.commit(commit) + if self.kModulesFile not in pcommit.tree: + raise ValueError("Tree of commit %s did not contain the %s file" % (commit, self.kModulesFile)) # END handle exceptions - self._root_tree = tree + self._parent_commit = pcommit - # clear the possibly changing values - del(self.path) - del(self._ref) - del(self._url) + # clear the possibly changed values + for name in ('path', '_ref', '_url'): + try: + delattr(self, name) + except AttributeError: + pass + # END for each name to delete def config_writer(self): """:return: a config writer instance allowing you to read and write the data @@ -108,11 +131,10 @@ def url(self): """:return: The url to the repository which our module-repository refers to""" return self._url - def root_tree(self): - """:return: Tree instance referring to the tree which contains the .gitmodules file - we are to use - :note: will always point to the current head's root tree if it was not set explicitly""" - return self._root_tree + def parent_commit(self): + """:return: Commit instance with the tree containing the .gitmodules file + :note: will always point to the current head's commit if it was not set explicitly""" + return self._parent_commit def config_reader(self): """:return: ConfigReader instance which allows you to qurey the configuration values diff --git a/test/git/test_config.py b/test/git/test_config.py index 604a25f42..8c846b996 100644 --- a/test/git/test_config.py +++ b/test/git/test_config.py @@ -14,9 +14,7 @@ class TestBase(TestCase): def _to_memcache(self, file_path): fp = open(file_path, "r") - sio = StringIO.StringIO() - sio.write(fp.read()) - sio.seek(0) + sio = StringIO.StringIO(fp.read()) sio.name = file_path return sio diff --git a/test/git/test_submodule.py b/test/git/test_submodule.py index 7922db77e..7c8dffcbe 100644 --- a/test/git/test_submodule.py +++ b/test/git/test_submodule.py @@ -19,7 +19,9 @@ def _do_base_tests(self, rwrepo): # size is invalid self.failUnlessRaises(ValueError, getattr, sm, 'size') - # fails if tree has no gitmodule file + # set_parent_commit fails if tree has no gitmodule file + + if rwrepo.bare: # module fails @@ -28,6 +30,8 @@ def _do_base_tests(self, rwrepo): # get the module repository pass # END bare handling + + # Writing of historical submodule configurations must not work @with_rw_repo(kCOTag) def test_base_rw(self, rwrepo):