Update 2 dependencies #1253

mtrmac · 2025-03-14T01:17:06Z

github.com/santhosh-tekuri/jsonschema/v5 is about 2 years old, github.com/russross/blackfriday more than 4 years old. Keeping up will only get harder as the users stay behind.

Note a possible downside: The v6 of the JSON schema parser imports golang.org/x/text to support localized error messages, and that is +17570 of a dependency.

Warning: I am not an expert in either of the packages, I was looking for the most similar new APIs. (I did manually verify that the markdown parsing produces the same code examples.)

tianon · 2025-03-20T17:28:02Z

As we just discussed in the OCI weekly meeting, Go version updates always make me nervous, especially with how many people import github.com/opencontainers/image-spec just for our struct/const definitions (which have essentially no Go version requirement and probably work all the way back to like Go 1.0 or earlier).

One idea we discussed was perhaps moving these deps/constraints to a new go.mod under schema (ie, create schema/go.mod) -- this would effectively create a new github.com/opencontainers/image-spec/schema "module", but the imports would all be exactly the same and go mod tidy should just DTRT for fixing consumers. 🤔

(Doing that would resolve my concerns pretty well, since the schema validation code has more valid reasons for enforcing a specific Go version than the struct/const definitions do.)

mtrmac · 2025-05-16T21:21:41Z

For a preview, this is what splitting out a …/schema module would look like.

Note the process in https://go.dev/wiki/Modules#is-it-possible-to-add-a-module-to-a-multi-module-repository : the next release of this repository would have to add tags, and a new inter-module dependency. I couldn’t test all of that directly, that would remain a lingering disk for the next release.

.github/workflows/docs-and-linting.yml

sudo-bmitch

I think a lot of the issues you're encountering would be solved with a go.work file. That shouldn't be committed to the repo, but can be setup in the GHA so that the CI jobs are running the tests against a consistent state.

sudo-bmitch · 2025-05-16T23:49:54Z

.github/workflows/docs-and-linting.yml

+        # current Go releases plus the version in the schema/go.mod are tested.
+        # Ideally we would also want to test with 1.18 for the top-level go.mod,
+        # but that doesn't work with the newer schema subdirectory.
+        go: ['1.21', 'oldstable', 'stable']


Lets revert this back to the original values. The go.mod file used can be changed in the setup-go step. In the render and lint step, initialize the go.work file.

Now proposing to test 1.18 in addition to the current 1.21, 1.23 and 1.24.

sudo-bmitch · 2025-05-16T23:51:12Z

RELEASES.md

@@ -90,6 +90,7 @@ $sig
 - [ ] edit/update the pull-request to link to the VOTE thread, from <https://groups.google.com/a/opencontainers.org/forum/#!forum/dev>
 - [ ] a week later, if the vote passes, merge the PR
  - [ ] `git tag -s $version $versionBumpCommit`
+  - [ ] `git tag -s schema/$version $versionBumpCommit`


I don't think this is needed. The tag at the top level will apply to the whole repo. The only issue with that is the schema/go.mod needs to refer to the version of the image-spec. Tagging that spec sets a hash, that hash needs to be in the go.sum, and that file needs to be part of the tagged content, so there's a circular dependency. We might want to solve that by pinning to a preceding commit rather than the tagged release.

I read https://go.dev/ref/mod#vcs-version to mean that a tag starting with the “module subdirectory” must exist, but I didn’t test that.

The circular dependency when tagging releases is fair enough; as you say, that can be solved by requiring a preceding commit instead.

The hassle primarily happens when adding features:

One option is to use replace or go.work. In either case, CI is not precisely representative of out-of-repo users: if a feature is added to image-spec/specs-go/v1, and a user to image-spec/schema without updating schema/go.mod, tests in this repo will pass, but external users might see failures (immediately when working against the development branch, or, later, if the release tagging process is not done correctly).

Another option is to do nothing special in CI; in that case, a feature would need to be implemented happen as two separate commits (or even separate PRs?) so that schema/go.mod can refer to the a commit hash adding the feature.

We discussed this a bit in today's community call. IMO we shouldn't over-rotate on tagging schema/vXXX for now, especially since that's something we can start doing in the future if it becomes important/worthwhile to do so.

The context for my position here is equal parts how much we (the maintainers) are collectively maintaining this library, and the number of people (publicly) using it: https://pkg.go.dev/github.com/opencontainers/image-spec/schema?tab=importedby (a tiny list, and it's mostly forks of image-spec or image-tools, with one external user).

So what I propose is that we do this (make schema a separate module), but for now we leave it untagged/unversioned. Personally I'm not convinced there are enough consumers of this code to justify doing any more than that, and I don't fully understand what value (or perceived value) the consumers of this code are receiving to make it worth more effort than that, so having that understanding would help convince me otherwise.

I think your solution with ad-hoc go work is solid for solving the CI use case.

So what I propose is that we [make schema a separate module], but for now we leave it untagged/unversioned.

Works for me.

sudo-bmitch · 2025-05-16T23:52:50Z

schema/go.mod

+
+require golang.org/x/text v0.14.0 // indirect
+
+replace github.com/opencontainers/image-spec => ../


Replace should only be used in development, and in this scenario, I think a go.work is a better solution. To implement:

cd .. go work init go work use . go work use schema

Note that the go.work file should not be checked into the repo, so it would be added to the .gitignore, and the beginning of the GHA jobs can recreate it.

This doesn’t quite work, at least right now, while the PR is in flight:

$ (cd schema && go mod tidy) go: github.com/opencontainers/image-spec/schema: ambiguous import: found package github.com/opencontainers/image-spec/schema in multiple modules: github.com/opencontainers/image-spec v1.1.0 (…/github.com/opencontainers/[email protected]/schema) github.com/opencontainers/image-spec/schema (…/github.com/opencontainers/image-spec/schema)

… but go test does succeed, so maybe that’s enough.

After there is a commit on the main branch, hopefully the dependency on the primary image-spec repo can be updated to a commit where image-spec/schema is a separate module, solving this.

mtrmac · 2025-05-20T21:59:13Z

Updated to newly test that the spec repo is consumable by Go 1.18, and to use go.work instead of replace.

tianon · 2025-05-23T00:08:14Z

schema/go.mod

+// The minimum Go release is only incremented when required by a feature.
+// At least 3 Go releases will be supported by the spec.
+// For example, updating this version to 1.19 first requires Go 1.21 to be released.


This comment shouldn't be needed here -- the point of splitting this module is that we can be more aggressive about upgrading the required Go version on the schema module. 👍

Dropped.

(For our use case, it is only important to keep the code working against the N-1 version of Go, and the CI already enforces that.)

tianon · 2025-05-23T00:10:22Z

.github/workflows/docs-and-linting.yml

-        go: ['go.mod', 'oldstable', 'stable']
+        # Current Go releases plus the version in go.mod and schema/go.mod are tested.
+        # See also BUILD_SPEC_MODULE_ONLY below.
+        go: ['1.18', '1.21', 'oldstable', 'stable']


The go.mod value here was actually really intentional to source the value directly from go.mod (see the conditionals below in the setup-go action usage).

Are there very many reasons for us to test the version in the top-level go.mod, or should we just use schema/go.mod for the "go.mod version" case and call it a day? I think identity/chainid_test.go is the only _test file outside schema, so I don't think we're getting much extra coverage by testing on 1.18 explicitly (except added complexity in the Makefile).

Are there very many reasons for us to test the version in the top-level go.mod

If an explicit goal is to keep the code compatible with Go 1.18, I think that should be enforced by a test; otherwise features from newer language versions are likely to creep in.

For that purpose, a go test ./... that triggers a build is ~sufficient, even if there are no unit tests.

tianon · 2025-05-23T00:12:18Z

.github/workflows/docs-and-linting.yml

@@ -28,17 +29,19 @@ jobs:
        with:
          go-version: ${{ matrix.go != 'go.mod' && matrix.go || null }}
          go-version-file: ${{ matrix.go == 'go.mod' && 'go/src/github.com/opencontainers/image-spec/go.mod' || null }}


If we do want to test both go.mod versions (and maybe even if we don't?) this could switch to something like endsWith(matrix.go, 'go.mod') and then use the value of matrix.go as the file directly (even better with #1264).

Thanks, done.

…module Signed-off-by: Miloslav Trmač <[email protected]>

This requires adding a $schema field to defs-descriptor.json, otherwise the "format": "uri" restriction was is ignored. Signed-off-by: Miloslav Trmač <[email protected]>

Signed-off-by: Miloslav Trmač <[email protected]>

mtrmac requested review from jonjohnsonjr, stevvooe, sudo-bmitch, sajayantony, tianon and cyphar as code owners March 14, 2025 01:17

mtrmac force-pushed the schemav6 branch 2 times, most recently from 99b1048 to d17ba45 Compare May 16, 2025 21:04

mtrmac commented May 16, 2025

View reviewed changes

.github/workflows/docs-and-linting.yml Outdated Show resolved Hide resolved

sudo-bmitch requested changes May 17, 2025

View reviewed changes

sudo-bmitch mentioned this pull request May 17, 2025

Chore: Remove GOPATH from GitHub Actions #1264

Open

mtrmac force-pushed the schemav6 branch 3 times, most recently from e788498 to 67cafc1 Compare May 20, 2025 21:54

tianon reviewed May 23, 2025

View reviewed changes

mtrmac added 3 commits May 26, 2025 20:28

Split github.com/opencontainers/image-spec/schema into a separate Go …

a89d2e4

…module Signed-off-by: Miloslav Trmač <[email protected]>

Update to github.com/santhosh-tekuri/jsonschema/v6

f9f9fd2

This requires adding a $schema field to defs-descriptor.json, otherwise the "format": "uri" restriction was is ignored. Signed-off-by: Miloslav Trmač <[email protected]>

Update to github.com/russross/blackfriday/v2

bbfff4d

Signed-off-by: Miloslav Trmač <[email protected]>

mtrmac force-pushed the schemav6 branch from 67cafc1 to bbfff4d Compare May 26, 2025 18:33


		require golang.org/x/text v0.14.0 // indirect

		replace github.com/opencontainers/image-spec => ../

Update 2 dependencies #1253

Are you sure you want to change the base?

Update 2 dependencies #1253

Uh oh!

Conversation

mtrmac commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tianon commented Mar 20, 2025

Uh oh!

mtrmac commented May 16, 2025

Uh oh!

Uh oh!

sudo-bmitch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac commented May 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mtrmac commented Mar 14, 2025 •

edited

Loading