close the release review: refuse two platforms sharing an output, a platform named twice, stray offer arguments and --notes with an empty value, and check both build files before migrate moves anything (3.0.83)

@@ -1 +1 @@1-3.0.821+3.0.83

@@ -96,10 +96,15 @@9696 	if platform_list.len == 0 {9797 		return error('${config_name} names no platforms')9898 	}99+	mut seen := map[string]bool{}99100 	for platform in platform_list {100101 		if platform !in platforms {101102 			return error('${config_name} names platform "${platform}", which is not one of ${platforms.join(', ')}')102103 		}104+		if seen[platform] {105+			return error('${config_name} names platform "${platform}" twice')106+		}107+		seen[platform] = true103108 	}104109 	return Config{105110 		command:   command

@@ -19,7 +19,12 @@1919 		values: ['notes']2020 		flags:  ['json', 'builds']2121 	}) or { return usage_error(err.msg()) }22-	if args.value('notes') != '' && !args.flag('builds') {22+	// `--notes ""` is still `--notes`, so the flag's presence decides, not its23+	// value; and offer takes no arguments at all.24+	if args.positional.len > 0 {25+		return usage_error('offer takes no arguments, not "${args.positional_at(0)}"')26+	}27+	if args.value_or_none('notes') != none && !args.flag('builds') {2328 		return usage_error('--notes belongs to --builds')2429 	}2530 	store := open_store() or { return refuse(args.json(), err.msg()) }

@@ -91,6 +91,7 @@9191 	}9292 	line := ancestry(manifests, head)!9393 	check_clean_tree(root, head)!94+	check_build_files(root)!9495 9596 	mut report := Report{9697 		root: root@@ -417,6 +418,15 @@417418 	}418419 }419420 421+// check_build_files runs in the check phase, before the swap: a repo holding422+// both build files is refused while nothing has moved, never left half done.423+fn check_build_files(root string) ! {424+	if os.is_file(os.join_path(root, kepr_build_file))425+		&& os.exists(os.join_path(root, build.config_name)) {426+		return error('both ${kepr_build_file} and ${build.config_name} exist — keep the one you want and migrate again')427+	}428+}429+420430 // rename_build_file finishes the move to `koh.build`, and records it as a save:421431 // the migration must not leave the working tree dirty with a rename the next422432 // save would otherwise pick up as unreviewed work.@@ -426,9 +436,6 @@426436 		return427437 	}428438 	fresh := os.join_path(root, build.config_name)429-	if os.exists(fresh) {430-		return error('both ${kepr_build_file} and ${build.config_name} exist — keep the one you want and migrate again')431-	}432439 	os.mv(old, fresh, os.MvParams{})!433440 	mut store := storage.open(root)!434441 	defer {

@@ -196,6 +196,39 @@196196 	assert snapshot.same_tree(history.files_of(store, head.id)!, files)197197 }198198 199+// A repo holding both build files is refused in the check phase, while the200+// Koh 2 data is still where it was.201+fn test_migration_refuses_two_build_files_without_moving_anything() {202+	koh2 := koh2_binary() or { return }203+	fixture := make_fixture(koh2)!204+	defer {205+		os.rmdir_all(fixture.dir) or {}206+	}207+	os.write_file(os.join_path(fixture.dir, 'kepr.build'), 'command = true\n')!208+	os.write_file(os.join_path(fixture.dir, 'koh.build'), 'command = true\n')!209+	sh(fixture.dir, '${koh2} save "both build files"')!210+	if _ := run(fixture.dir, default_lane) {211+		assert false, 'both build files must be refused'212+	} else {213+		assert err.msg().contains('both kepr.build and koh.build')214+	}215+	assert os.is_dir(os.join_path(fixture.dir, '.koh'))216+	assert !os.exists(os.join_path(fixture.dir, '.koh-v2'))217+}218+219+// A repo with no Koh 2 build file migrates exactly as it always did.220+fn test_migration_without_a_build_file_is_unchanged() {221+	koh2 := koh2_binary() or { return }222+	fixture := make_fixture(koh2)!223+	defer {224+		os.rmdir_all(fixture.dir) or {}225+	}226+	report := run(fixture.dir, default_lane)!227+	assert !report.build_renamed228+	assert !os.exists(os.join_path(fixture.dir, 'koh.build'))229+	assert os.is_dir(os.join_path(fixture.dir, '.koh-v2'))230+}231+199232 fn test_migrate_refuses_a_tree_that_does_not_match_the_last_save() {200233 	koh2 := koh2_binary() or { return }201234 	fixture := make_fixture(koh2)!

@@ -69,8 +69,16 @@6969 	config build.Config, version string, notes string) !Published {7070 	head := history.head(store) or { return error(exchange.no_saves) }7171 	mut built := []build.Built{}72+	mut paths := map[string]bool{}7273 	for platform in config.platforms {73-		built << build.build_one(store.root, config, platform, version)!74+		artifact := build.build_one(store.root, config, platform, version)!75+		// Every build happens before the first upload, so two platforms landing76+		// on one path would publish the second build's bytes for both.77+		if artifact.path in paths {78+			return error('two platforms build to the same file, ${artifact.path} — name the platform in ${build.config_name}\'s output, as "out/demo-{platform}" does')79+		}80+		paths[artifact.path] = true81+		built << artifact7482 	}7583 	mut files := []File{}7684 	for artifact in built {

@@ -799,6 +799,53 @@799799 	assert run(dir, 'log').out.contains('checkpoint save')800800 }801801 802+// Publishing refuses what it cannot do before it builds anything, and a real803+// publish writes the layout §11.2 names. The command lines are checked first:804+// a mistake there is a usage error, not a refusal.805+fn test_offer_builds_refuses_bad_command_lines_and_a_dirty_tree() {806+	dir := temp_repo('offer_builds')807+	defer { os.rmdir_all(dir) or {} }808+	init_repo(dir)809+	home := temp_repo('offer_builds_home')810+	defer { os.rmdir_all(home) or {} }811+	name := os.base(dir)812+	notes := 'a release note long enough to clear forty characters and the twelve word rule easily'813+	assert run(dir, 'offer', '--notes', notes).code == 2814+	assert run(dir, 'offer', '--notes', '').code == 2815+	assert run(dir, 'offer', 'extra').code == 2816+817+	write(dir, 'VERSION', '1.2.3\n')818+	write(dir, 'koh.build', 'command = mkdir -p out && printf x > out/demo-1.2.3-${this_platform()}819+output = out/demo-{version}-{platform}820+platforms = ${this_platform()}821+')822+	assert run(dir, 'save', 'first').code == 0823+	remote := os.join_path(home, 'remote')824+	assert run_home(dir, home, 'remote', 'folder', remote).code == 0825+826+	// A dirty tree cannot publish: the files must belong to a save.827+	write(dir, 'extra.txt', 'unsaved\n')828+	dirty := run_home(dir, home, 'offer', '--builds')829+	assert dirty.code == 1830+	assert dirty.err.contains('save or checkpoint first')831+832+	// Notes that say nothing are refused before anything is built or uploaded.833+	assert run_home(dir, home, 'save', 'second').code == 0834+	short := run_home(dir, home, 'offer', '--builds', '--notes', 'too short')835+	assert short.code == 1836+	assert short.err.contains('at least 40 characters')837+	assert !os.exists(os.join_path(remote, name, 'releases'))838+839+	published := run_home(dir, home, 'offer', '--builds', '--notes', notes)840+	assert published.code == 0, published.err841+	artifact := 'demo-1.2.3-${this_platform()}'842+	assert os.is_file(os.join_path(remote, name, 'releases/1.2.3/release.json'))843+	assert os.is_file(os.join_path(remote, name, 'releases/1.2.3/${this_platform()}/${artifact}'))844+	assert os.read_file(os.join_path(remote, name, 'releases/latest'))! == '1.2.3'845+	record := os.read_file(os.join_path(remote, name, 'releases/1.2.3/release.json'))!846+	assert record.contains('"notes":"${notes}"')847+}848+802849 fn test_remote_configuration_is_isolated_and_atomic() {803850 	dir := temp_repo('remote')804851 	home := temp_repo('remote-home')

@@ -1,6 +1,6 @@11 Module {22 	name: 'koh3'3-	version: '3.0.82'3+	version: '3.0.83'44 	description: 'One SQLite file per repo, one line of history.'55 	license: 'MIT'66 }