Skip to content

Commit afbf151

Browse files
committed
Fix how seeding process deals with release artefacts.
There are several problems with the way the seeding process (1) copies the release artefacts from the src/ontology directory to the top-level release directory and (2) optionally commits those files. For (1), the problem is that the seeding script manually copies the files itself, and do so by assuming too many things about those files. Notably, it assumes they all go straight to the top-level directory, whereas some should actually go a subdirectory (for example, report files, when they are released, go into $(RELEASEDIR)/reports, not into $(RELEASEDIR) directly; so do subset files). The problem with (2) is a consequence of (1): if not all release files have been correctly copied to their correct expected location in the top-level directory, then the seeding script will try to `git add` files that may not actually exist. We fix (1) by adding a new helper target, `copy_release_files`, to take care of copying all release files to the release directory. It is called internally by the `prepare_release` target, and also by the seeding script. This ensures that the same logic to copy release files will always be used. Wr fix (2) by updating the `show_release_assets` target (whose sole real purpose is to be used by the seeding script) to list the _actual_ release files: that is, the files that are in the release directory, rather than the files under src/.
1 parent 787a8ac commit afbf151

2 files changed

Lines changed: 37 additions & 14 deletions

File tree

odk/odk.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,11 +1464,11 @@ def seed(config, clean, outdir, templatedir, dependencies, title, user, source,
14641464
format(dir=outdir,
14651465
branch=project.git_main_branch,
14661466
files=" ".join([t.replace(outdir, ".", 1) for t in tgts])))
1467-
runcmd("cd {dir}/src/ontology && make all_assets && cp $(make show_release_assets) ../../"
1467+
runcmd("cd {dir}/src/ontology && make all_assets copy_release_files"
14681468
.format(dir=outdir))
14691469
if commit_artefacts:
14701470
runcmd("cd {dir}/src/ontology "
1471-
"&& for asset in $(make show_release_assets) ; do git add -f ../../$asset ; done"
1471+
"&& for asset in $(make show_release_assets) ; do git add -f $asset ; done"
14721472
.format(dir=outdir))
14731473
runcmd("cd {dir} && if [ -n \"$(git status -s)\" ]; then git commit -a -m 'initial build' ; fi"
14741474
.format(dir=outdir))

template/src/ontology/Makefile.jinja2

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,15 @@ ASSETS = \
394394
$(SUBSET_FILES) \
395395
$(MAPPING_FILES)
396396

397+
{# This does not include mappings (which can optionally be released),
398+
because those are stored in src/mappings, and the fact that this is
399+
one directory _above_ the directory where this Makefile is would
400+
cause problems down the road (notably, it would prevent the use of a
401+
single rsync commmand to copy all release assets except mappings to
402+
the release directory). Basically, this is "all release assets that
403+
are in this src/ontology directory".
404+
This also does not include pattern-derived files, and it is unclear
405+
whether this is by design or not, see #1243. -#}
397406
RELEASE_ASSETS = \
398407
$(MAIN_FILES) {% if project.import_group is defined %}{% if project.import_group.release_imports %}$(IMPORT_FILES) {% endif %}{% endif %}\
399408
$(SUBSET_FILES){% if project.robot_report.release_reports %} \
@@ -407,10 +416,6 @@ show_assets:
407416
echo $(ASSETS)
408417
du -sh $(ASSETS)
409418

410-
.PHONY: show_release_assets
411-
show_release_assets:
412-
@echo $(RELEASE_ASSETS)
413-
414419
check_rdfxml_%: %
415420
@check-rdfxml {% if project.extra_rdfxml_checks %}--jena --rdflib{% endif %} $<
416421

@@ -426,22 +431,41 @@ KEEPRELATIONS=keeprelations.txt
426431
{% endif -%}
427432

428433
CLEANFILES=$(MAIN_FILES) $(SRCMERGED) $(EDIT_PREPROCESSED)
434+
429435
# This should be executed by the release manager whenever time comes to make a release.
430436
# It will ensure that all assets/files are fresh, and will copy to release folder
431-
432437
.PHONY: prepare_release
433-
prepare_release: all_odk
434-
rsync -R $(RELEASE_ASSETS) $(RELEASEDIR) &&\{% if project.sssom_mappingset_group is defined %}{% if project.sssom_mappingset_group.released_products is defined %}
435-
mkdir -p $(RELEASEDIR)/mappings && cp -rf $(RELEASED_MAPPING_FILES) $(RELEASEDIR)/mappings &&\{% endif %}{% endif %}{% if project.use_dosdps %}
436-
mkdir -p $(RELEASEDIR)/patterns && cp -rf $(PATTERN_RELEASE_FILES) $(RELEASEDIR)/patterns &&\{% endif %}
437-
rm -f $(CLEANFILES) &&\
438-
echo "Release files are now in $(RELEASEDIR) - now you should commit, push and make a release \
438+
prepare_release: all_odk copy_release_files
439+
rm -f $(CLEANFILES)
440+
@echo "Release files are now in $(RELEASEDIR) - now you should commit, push and make a release \
439441
on your git hosting site such as GitHub or GitLab"
440442

441443
.PHONY: prepare_release_fast
442444
prepare_release_fast:
443445
$(MAKE) prepare_release IMP=false PAT=false MIR=false COMP=false
444446

447+
# This rule does the bulk of the work for prepare_release, copying all
448+
# files to the release directory. It is mostly intended for internal
449+
# use, by the prepare_release rule itself or by other ODK scripts.
450+
.PHONY: copy_release_files
451+
copy_release_files:
452+
rsync -R $(RELEASE_ASSETS) $(RELEASEDIR){% if project.sssom_mappingset_group is defined %}{% if project.sssom_mappingset_group.released_products is defined %}
453+
mkdir -p $(RELEASEDIR)/mappings
454+
cp -rf $(RELEASED_MAPPING_FILES) $(RELEASEDIR)/mappings{% endif %}{% endif %}{% if project.use_dosdps %}
455+
mkdir -p $(RELEASEDIR)/patterns
456+
cp -rf $(PATTERN_RELEASE_FILES) $(RELEASEDIR)/patterns{% endif %}
457+
458+
# All released assets, in their final location within the release
459+
{# ODK devs: keep that in sync with the copy_release_files rule above
460+
-- that is, this should list all files copied by that rule (except
461+
the pattern-derived files, whose status is unclear; see #1243). -#}
462+
RELEASE_ASSETS_AFTER_RELEASE=$(foreach n,$(RELEASE_ASSETS), $(RELEASEDIR)/$(n)){% if project.sssom_mappingset_group is defined %}{% if project.sssom_mappingset_group.released_products is defined %} \
463+
$(foreach n,$(RELEASED_MAPPINGS), $(RELEASEDIR)/mappings/$(n).sssom.tsv){% endif %}{% endif %}
464+
465+
.PHONY: show_release_assets
466+
show_release_assets:
467+
@echo $(RELEASE_ASSETS_AFTER_RELEASE)
468+
445469
CURRENT_RELEASE=$(ONTBASE).owl
446470

447471
$(TMPDIR)/current-release.owl:
@@ -1493,7 +1517,6 @@ public_release:
14931517
$(GITHUB_RELEASE_PYTHON) --release $(TAGNAME) $(RELEASEFILES)
14941518
{% else %}
14951519

1496-
RELEASE_ASSETS_AFTER_RELEASE=$(foreach n,$(RELEASE_ASSETS), $(RELEASEDIR)/$(n)) $(foreach n,$(RELEASED_MAPPINGS), $(RELEASEDIR)/mappings/$(n).sssom.tsv)
14971520
GHVERSION=v$(VERSION)
14981521

14991522
.PHONY: public_release

0 commit comments

Comments
 (0)