twoliter: Remove Release.toml from variant build #112

ecpullen · 2023-11-14T19:54:27Z

Issue number:

Closes #109

Description of changes:

Instead of using Release.toml to determine the version, use the version listed in the variants Cargo.toml instead.

Testing done:

Unit test added to tests/variant_version
Integ test with project1

$ cargo run --manifest-path /home/ec2-user/twoliter/Cargo.toml -p twoliter build variant  hello-ootb

[cargo-make] INFO - Running Task: publish-setup
19:48:02 [INFO] No infra config at '/home/ec2-user/twoliter/tests/projects/project1/Infra.toml' - using local roles/keys
[cargo-make] INFO - Running Task: build-variant
   Compiling hello-ootb v0.1.0 (/home/ec2-user/twoliter/tests/projects/project1/variants/hello-ootb)
    Finished dev [optimized] target(s) in 1m 07s
[cargo-make] INFO - Build Done in 77.66 seconds.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

webern · 2023-11-20T18:13:45Z

Rebase me!

webern

Can't comment on twoliter/src/cmd/new.rs which appears to be extraneous in this PR.

twoliter/src/variant.rs

webern · 2023-11-21T01:21:36Z

twoliter/src/variant.rs

+    serde_json::from_str::<Value>(
+        &exec(
+            Command::new("cargo").args([
+                "metadata",


@bcressey we discussed a few ideas about where we should put image (i.e. variant) versions. There could be a single one for all variants in Twoliter.toml, variants could be enumerated and versioned in Twoliter.toml, we could put it in the variant's custom Cargo.toml metadata...

Here we are using the actual cargo package version as the variant's version. What do you think?

Note, we are doing this now to remove the Release.toml requirement.

Here we are using the actual cargo package version as the variant's version. What do you think?

My biggest gripe is that it's inconsistent with the cargo manifests for packages, where the crate version is ignored and only the version in the package's spec file has any external effect. As a minor nit, I also don't love parsing the crate manifest in different ways with different tools; we already have a parser in buildsys and could factor that out into a standalone crate if we needed to.

A single version in Twoliter.toml is the most consistent with existing logic and would make what I consider to be the common case easier - a project where multiple variants are built and released together, all tied to a specific Git commit tagged with a version. Updating a dozen versions across all variant manifests would be a lot of boilerplate for a routine chore, vs. just editing the top-level file.

I am still somewhat interested in the direction where variants (and kits) all have associated RPM spec files, to include package dependencies via Requires and stray files that don't make sense elsewhere. If we did that we could get the variant's version from the spec file, which would be consistent with package versioning.

But I'm not convinced that would be better. Before we added application-inventory.json, none of these internal versions had any external significance. The spec file versions were mostly there as an implementation detail and a nod to spec file conventions. They're also a bit annoying to maintain since every package update requires at least two files to edit.

My preference would be to start with a top-level release-version field in Twoliter.toml, and if we need variant-specific versions for some reason, we could add a release-version-override field to package.metadata.build-variant.

webern · 2023-11-21T01:24:53Z

twoliter/embedded/Makefile.toml

@@ -137,6 +137,8 @@ TESTSYS_LOG_LEVEL = "info"
 # Certain variables are defined here to allow us to override a component value
 # on the command line.

+BUILDSYS_VERSION_IMAGE = { script = ["awk -F '[ =\"]+' '$1 == \"version\" {print $2}' ${BUILDSYS_RELEASE_CONFIG_PATH}"], condition = { env_not_set = ["BUILDSYS_VERSION_IMAGE"]}}


I think we could require this to be set instead of discovering it from Release.toml here. This code is duplicated in Bottlerocket's Makefile.toml which means that Bottlerocket's build is always going to be setting this.

I think we could require this to be set instead of discovering it from Release.toml here. This code is duplicated in Bottlerocket's Makefile.toml which means that Bottlerocket's build is always going to be setting this.

Note, we discussed, in twoliter make we can look for BUILDSYS_VERSION_IMAGE, if it's not there or empty, we can get it from Twoliter.toml and set it before calling cargo make.

local/README.md

bcressey · 2023-11-28T21:34:08Z

twoliter/src/common.rs

-/// `quiet=true`, no output will be shown.
-pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<()> {
+/// `quiet=true`, no output will be shown and will be returned instead.
+pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<String> {


Rather than sometimes returning an empty string, I'd prefer to return a Result<Option<String>>.

webern · 2023-11-30T22:31:39Z

https://github.com/bottlerocket-os/twoliter/compare/7b0123f22840fd2275ac7a58965676cee73f6124..65b153ae78c3b76d7bd08bc2da7d2ff8bc63287e

Rebase.

webern · 2023-12-01T02:32:12Z

https://github.com/bottlerocket-os/twoliter/compare/249571779ab10b569546d86194c81bd58dbffd7d..ff6125c786f12f25f3ecf0362a5332d01972eec3

Broke the README change out to its own commit.
Added release-version to Twoliter.toml
Created a validating system when loading Twoliter.toml than can issue a warning about Release.toml and make sure it has the same version as Twoliter.toml.
Updated Ethan's commit to take the version from Twoliter.toml

TODO: Probably should add tests of the Release.toml validation.

webern · 2023-12-01T02:38:52Z

https://github.com/bottlerocket-os/twoliter/compare/ff6125c786f12f25f3ecf0362a5332d01972eec3..fd9907d6abeb3e8c1c962f92c5290dcb54e63e06

Improve a comment.

ecpullen · 2023-12-01T17:56:17Z

local/README.md

@@ -1,3 +1,55 @@
+# Building The Alpha SDK


This file can be removed. It wasn't supposed to be a part of this branch.

bcressey · 2023-12-05T19:29:55Z

twoliter/src/project.rs

+        warn!(
+            "A Release.toml file was found. Release.toml is deprecated. Please remove it from \
+             your project."
+        );


nit: this could encourage people building from the main repo to send a PR to remove Release.toml - might want to drop it.

When loading a project, issue a deprecation warning if Release.toml is found. If it is found, ensure that its version field is aligned with Twoliter.toml

It is no longer required to have a Release.toml file in the project. Co-authored-by: Ethan Pullen <[email protected]> Co-authored-by: Matthew James Briggs <[email protected]>

webern · 2023-12-07T18:37:53Z

twoliter/embedded/Makefile.toml

@@ -15,9 +15,13 @@ BUILDSYS_IMAGES_DIR = "${BUILDSYS_BUILD_DIR}/images"
 BUILDSYS_TOOLS_DIR = "${BUILDSYS_ROOT_DIR}/tools"
 BUILDSYS_SOURCES_DIR = "${BUILDSYS_ROOT_DIR}/sources"
 BUILDSYS_SBKEYS_DIR = "${BUILDSYS_ROOT_DIR}/sbkeys"
-BUILDSYS_SBKEYS_PROFILE = { script = ['echo "${BUILDSYS_SBKEYS_PROFILE:-local}"'] }
+BUILDSYS_SBKEYS_PROFILE = { script = [


Please undo your IDE's auto-reformatting of this file.

We had a mistake in bottlerocket-os#112 commit 67ef875. The check to make sure the version Release.toml matched the release version in Twoliter.toml could never succeed due to quotation marks around the version string. This fixes it and adds a test to prove that it works.

ecpullen force-pushed the remove-releasetoml-variant branch from e1686f2 to 3ce1844 Compare November 14, 2023 20:00

ecpullen force-pushed the remove-releasetoml-variant branch from 3ce1844 to d2dbe85 Compare November 20, 2023 18:33

webern reviewed Nov 21, 2023

View reviewed changes

webern mentioned this pull request Nov 22, 2023

tracking issue: release v0.0.5 #117

Closed

7 tasks

ecpullen force-pushed the remove-releasetoml-variant branch from d2dbe85 to 7b0123f Compare November 28, 2023 19:41

bcressey reviewed Nov 28, 2023

View reviewed changes

webern force-pushed the remove-releasetoml-variant branch from 7b0123f to 65b153a Compare November 30, 2023 22:31

webern force-pushed the remove-releasetoml-variant branch 2 times, most recently from 2495717 to ff6125c Compare December 1, 2023 02:30

webern force-pushed the remove-releasetoml-variant branch from ff6125c to fd9907d Compare December 1, 2023 02:38

ecpullen commented Dec 1, 2023

View reviewed changes

bcressey approved these changes Dec 5, 2023

View reviewed changes

webern and others added 4 commits December 6, 2023 17:53

add release-version to twoliter.toml

48a9414

move schema version type to its own file

93b426d

deprecate release.toml

67ef875

When loading a project, issue a deprecation warning if Release.toml is found. If it is found, ensure that its version field is aligned with Twoliter.toml

twoliter: remove release.toml requirement

ba4185a

It is no longer required to have a Release.toml file in the project. Co-authored-by: Ethan Pullen <[email protected]> Co-authored-by: Matthew James Briggs <[email protected]>

ecpullen force-pushed the remove-releasetoml-variant branch from fd9907d to ba4185a Compare December 6, 2023 17:57

twoliter: User Twoliter.toml for version in make

8b48186

webern suggested changes Dec 7, 2023

View reviewed changes

twoliter: Make BUILDSYS_VERSION_IMAGE required

13493e8

ecpullen force-pushed the remove-releasetoml-variant branch from 328c851 to 13493e8 Compare December 7, 2023 19:55

ecpullen requested a review from webern December 7, 2023 20:08

webern approved these changes Dec 7, 2023

View reviewed changes

ecpullen merged commit e88ede9 into bottlerocket-os:develop Dec 8, 2023
1 check passed

webern mentioned this pull request Dec 8, 2023

fix release.toml version check #126

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

twoliter: Remove Release.toml from variant build #112

twoliter: Remove Release.toml from variant build #112

ecpullen commented Nov 14, 2023

webern commented Nov 20, 2023

webern left a comment

webern Nov 21, 2023 •

edited

Loading

bcressey Nov 28, 2023 •

edited

Loading

webern Nov 21, 2023

webern Dec 5, 2023

bcressey Nov 28, 2023

webern commented Nov 30, 2023

webern commented Dec 1, 2023 •

edited

Loading

webern commented Dec 1, 2023

ecpullen Dec 1, 2023

bcressey Dec 5, 2023

webern Dec 7, 2023

twoliter: Remove Release.toml from variant build #112

twoliter: Remove Release.toml from variant build #112

Conversation

ecpullen commented Nov 14, 2023

webern commented Nov 20, 2023

webern left a comment

Choose a reason for hiding this comment

webern Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

bcressey Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

webern Nov 21, 2023

Choose a reason for hiding this comment

webern Dec 5, 2023

Choose a reason for hiding this comment

bcressey Nov 28, 2023

Choose a reason for hiding this comment

webern commented Nov 30, 2023

webern commented Dec 1, 2023 • edited Loading

webern commented Dec 1, 2023

ecpullen Dec 1, 2023

Choose a reason for hiding this comment

bcressey Dec 5, 2023

Choose a reason for hiding this comment

webern Dec 7, 2023

Choose a reason for hiding this comment

webern Nov 21, 2023 •

edited

Loading

bcressey Nov 28, 2023 •

edited

Loading

webern commented Dec 1, 2023 •

edited

Loading