On Wed, Oct 17, 2012 at 01:48:14PM +0100, Sam Thursfield wrote: What about a release branch, where part of the release process is to brand your component with the release name, and this change doesn't go into the main line of development, so it's clear that any builds that aren't from a release are development versions. In this case it would be wasteful to have to create a branch for the branding change, then create another branch for releasing with the branding. For full development, certainly, but it is common to hack on master for a bit until you get an idea of what you're doing. I agree that it's not a common problem though, so I'm happy with the change. Keying strata by repository and morphology name worry me slightly, since in other places we treat the repo, ref and name as the unique key, which means we'd have a problem if something uses strata where their only difference is the ref. I do agree that this is a rare and stupid thing to do though, and petrifying as a whole is likely to be problematic in that case, so we can deal with that later. Is this right? It appears to warn that it's not merging, but still changes the ref of the stratum info. You could probably use locals() here instead of creating the dict, but it's probably better like this. PEP8 says to use parentheses rather than \ to handle line breaks in long expressions, but if it turns out to be the only problem I'm happy to fix that up in the merge.

This change causes 'morph petrify' to avoid petrifying any chunk whose ref matches the current system branch, because it makes no sense to petrify something that is also being edited. It also improves efficiency slightly and adds warning where different systems point to different refs of the same stratum. A non-obvious effect of this is that if you try to petrify 'master', many of the chunks won't get petrified because they are built from 'master'. However, petrifying master makes no sense so I'm not sure that we need to worry. --- morphlib/plugins/branch_and_merge_plugin.py | 57 ++++++++++++------ tests.branching/petrify-no-double-petrify.script | 4 +- tests.branching/petrify.script | 11 +++- tests.branching/petrify.stdout | 12 ++++ tests.branching/setup-second-chunk | 69 ++++++++++++++++++++++ 5 files changed, 130 insertions(+), 23 deletions(-) create mode 100755 tests.branching/setup-second-chunk diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py index 384f055..050410d 100644 --- a/morphlib/plugins/branch_and_merge_plugin.py +++ b/morphlib/plugins/branch_and_merge_plugin.py @@ -722,7 +722,8 @@ class BranchAndMergePlugin(cliapp.Plugin): Strata refs are not petrified, because they must all be edited to set the new chunk refs, which requires branching them all for the current branch - so they will not be updated outside of the user's control in - any case. + any case. Chunks that have already been edited on the current branch + are also not petrified. ''' @@ -733,9 +734,11 @@ class BranchAndMergePlugin(cliapp.Plugin): branch, branch_path = self.deduce_system_branch() root_repo = self.get_branch_config(branch_path, 'branch.root') root_repo_dir = self.find_repository(branch_path, root_repo) - self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app) + # We must first get the full set of strata. One same stratum may be + # in multiple systems and each system may use a different ref. + strata = {} for f in glob.glob(os.path.join(root_repo_dir, '*.morph')): name = os.path.basename(f)[:-len('.morph')] morphology = self.load_morphology(root_repo_dir, name) @@ -743,24 +746,42 @@ class BranchAndMergePlugin(cliapp.Plugin): continue for stratum_info in morphology['strata']: - repo_dir = self.edit_stratum( - branch, branch_path, root_repo, root_repo_dir, - stratum_info) - - stratum = self.load_morphology(repo_dir, stratum_info['morph']) - - for chunk_info in stratum['chunks']: - if 'unpetrify-ref' not in chunk_info: - commit_sha1, tree_sha1 = self.app.resolve_ref( - self.lrc, self.rrc, chunk_info['repo'], - chunk_info['ref'], - update=not self.app.settings['no-git-update']) - chunk_info['unpetrify-ref'] = chunk_info['ref'] - chunk_info['ref'] = commit_sha1 - self.save_morphology(repo_dir, stratum_info['morph'], stratum) - + key = (stratum_info['repo'], stratum_info['morph']) + if key in strata: + original_ref = strata[key] + if stratum_info['ref'] == branch: + strata[key] = branch + elif stratum_info['ref'] != original_ref: + if original_ref != branch: + self.app.output.write( + 'WARNING: not merging any differences from ' + 'ref %s into %s of stratum %s\n' % + (stratum_info['ref'], original_ref, + stratum_info['morph'])) + stratum_info['ref'] = branch + else: + strata[key] = stratum_info['ref'] + stratum_info['ref'] = branch self.save_morphology(root_repo_dir, name, morphology) + for (repo, morph), ref in strata.iteritems(): + repo_dir = self.edit_stratum( + branch, branch_path, root_repo, root_repo_dir, + { 'repo': repo, 'ref': ref, 'morph': morph}) + + stratum = self.load_morphology(repo_dir, morph) + + for chunk_info in stratum['chunks']: + if chunk_info['ref'] != branch and \ + 'unpetrify-ref' not in chunk_info: + commit_sha1, tree_sha1 = self.app.resolve_ref( + self.lrc, self.rrc, chunk_info['repo'], + chunk_info['ref'], + update=not self.app.settings['no-git-update']) + chunk_info['unpetrify-ref'] = chunk_info['ref'] + chunk_info['ref'] = commit_sha1 + self.save_morphology(repo_dir, morph, stratum) + self.print_changelog('The following changes were made but have not ' 'been comitted') diff --git a/tests.branching/petrify-no-double-petrify.script b/tests.branching/petrify-no-double-petrify.script index 3e7ac41..9fc00d3 100755 --- a/tests.branching/petrify-no-double-petrify.script +++ b/tests.branching/petrify-no-double-petrify.script @@ -31,9 +31,9 @@ fi cd "$DATADIR/workspace" "$SRCDIR/scripts/test-morph" init -"$SRCDIR/scripts/test-morph" checkout baserock:morphs master +"$SRCDIR/scripts/test-morph" branch baserock:morphs test/petrify -cd master/baserock:morphs +cd test/petrify/baserock:morphs "$SRCDIR/scripts/test-morph" petrify "$SRCDIR/scripts/test-morph" petrify "$SRCDIR/scripts/test-morph" petrify diff --git a/tests.branching/petrify.script b/tests.branching/petrify.script index 0b5ff01..c1828bf 100755 --- a/tests.branching/petrify.script +++ b/tests.branching/petrify.script @@ -15,7 +15,8 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -# Verify that "morph petrify" works. +# Verify "morph petrify", and ensure it doesn't petrify chunks that have +# already been edited. set -eu @@ -28,11 +29,15 @@ then exit 0 fi +. "$SRCDIR/tests.branching/setup-second-chunk" + cd "$DATADIR/workspace" "$SRCDIR/scripts/test-morph" init -"$SRCDIR/scripts/test-morph" checkout baserock:morphs master +"$SRCDIR/scripts/test-morph" branch baserock:morphs test/petrify master + +cd test/petrify/baserock:morphs +"$SRCDIR/scripts/test-morph" edit hello-system hello-stratum goodbye -cd master/baserock:morphs "$SRCDIR/scripts/test-morph" petrify echo "Petrified:" cat hello-stratum.morph diff --git a/tests.branching/petrify.stdout b/tests.branching/petrify.stdout index 249a8f5..6219c84 100644 --- a/tests.branching/petrify.stdout +++ b/tests.branching/petrify.stdout @@ -9,6 +9,12 @@ Petrified: "ref": "f4d032b42c0134e67bdf19a43fa99072493667d7", "build-depends": [], "unpetrify-ref": "master" + }, + { + "name": "goodbye", + "repo": "baserock:goodbye", + "ref": "test/petrify", + "build-depends": [] } ] } @@ -23,6 +29,12 @@ Unpetrified: "repo": "baserock:hello", "ref": "master", "build-depends": [] + }, + { + "name": "goodbye", + "repo": "baserock:goodbye", + "ref": "test/petrify", + "build-depends": [] } ] } diff --git a/tests.branching/setup-second-chunk b/tests.branching/setup-second-chunk new file mode 100755 index 0000000..9f7aa41 --- /dev/null +++ b/tests.branching/setup-second-chunk @@ -0,0 +1,69 @@ +#!/bin/sh +# Copyright (C) 2012 Codethink Limited +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + + +# Add a second chunk to hello-stratum. + +. "$SRCDIR/scripts/fix-committer-info" + +create_chunk() { + REPO="$1" + NAME="$2" + + mkdir "$1" + ln -s "$1" "$1.git" + cd "$1" + + cat <<EOF > "$1/$2.morph" +{ + "name": "$2", + "kind": "chunk", + "build-system": "dummy" +} +EOF + + git init --quiet + git add . + git commit --quiet -m "Initial commit" +} + +create_chunk "$DATADIR/goodbye" "hello" + +cd "$DATADIR/morphs" +cat <<EOF > hello-stratum.morph +{ + "name": "hello-stratum", + "kind": "stratum", + "chunks": [ + { + "name": "hello", + "repo": "baserock:hello", + "ref": "master", + "build-depends": [] + }, + { + "name": "goodbye", + "repo": "baserock:goodbye", + "ref": "master", + "build-depends": [] + } + ] +} +EOF + +git commit -q --all -m "Add goodbye to hello-stratum" + +cd "$DATADIR/workspace" -- 1.7.10.4