Update 2 dependencies by mtrmac · Pull Request #1253 · opencontainers/image-spec

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

 - [ ] 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/image-spec@v1.1.0/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.

mtrmac · 2025-07-04T15:37:21Z

Rebased, PTAL.

tianon

I don't love BUILD_SPEC_MODULE_ONLY, but it seems fine 👍

Thanks!

sudo-bmitch

schema/go.sum is missing the entry for the image-spec package, but adding it breaks things since then there's two references to schema (one from the imported 1.1.1 release). I think the only way to sort that out is to merge this and follow up with a separate PR that does a cd schema && go get github.com/opencontainers/image-spec@latest.

I'm not excited about the BUILD_SPEC_MODULE_ONLY either. It looks like a complication of supporting 1.18 in the go.mod which can't automatically update the go toolchain, and the GOTOOLCHAIN=local setting even if we did. I'd probably rename it SKIP_SCHEMA_CHECKS, but that's a personal preference.

@tianon if you're still in agreement after reading my review, you're welcome to merge it.

…module Signed-off-by: Miloslav Trmač <mitr@redhat.com>

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

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

This updates github.com/opencontainers/image-spec/schema subpackage from an obsolete .../jsonschema/v5 to /v6 . We use these dependencies only in tests, so it doesn't matter that much, but removing references to years-old unmaintained versions makes it easier to check / remove other such cases. (This is test-only, so the added files in vendor/ do not mean a larger binary size; it's almost exactly unchanged.) Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Update opencontainers/image-spec after opencontainers/image-spec#1253

Bumping skopeo to version v1.20.0, which comprises the following commits: e2c1eecd Bump to v1.20.0 33d3ae41 chore(deps): update module github.com/docker/docker to v28.3.3+incompatible [security] 36176ac4 fix(deps): update github.com/opencontainers/image-spec digest to 2daaaaf d1bbcb46 openshiftCluster.dockerLogin: create path with parents 5973d144 integration: rename registry binary a4f3fb31 Update dependency golangci/golangci-lint to v2.3.0 203a4cf7 Improve parameter passing to hack/test-system.sh 319d18c0 Improve parameter passing to hack/test-integration.sh 94e94998 Centralize policy edits in copySuite.policyFixture 5d2cb67f Use t.Cleanup in fileFromFixture d64b5991 Update module github.com/spf13/pflag to v1.0.7 0aab0745 Update module github.com/containers/common to v0.64.0 a3b8e175 Update opencontainers/image-spec after opencontainers/image-spec#1253 . f17b4c96 Update module github.com/containers/image/v5 to v5.36.0 f87d5696 Update dependency golangci/golangci-lint to v2.2.2 f9bdc2d8 Consolidate options shared between copy and sync to sharedCopyOptions 1f18641d Consistently use AddFlagSet before flag definitions 4ac60afc Update GPG keys not to use SHA-1 862c2331 Update module golang.org/x/term to v0.33.0 ef2375bc Update module github.com/Masterminds/semver/v3 to v3.4.0 3de83e9f Update dependency golangci/golangci-lint to v2.2.1 3c93577b Packit: disable official CentOS Stream update job ac8b6527 Update Neil Smith's GitHub username in MAINTAINERS.md Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>

mtrmac requested review from cyphar, jonjohnsonjr, sajayantony, stevvooe, sudo-bmitch and tianon 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

Merged

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 force-pushed the schemav6 branch from 67cafc1 to bbfff4d Compare May 26, 2025 18:33

mtrmac force-pushed the schemav6 branch 3 times, most recently from 6d28311 to 8384e0f Compare July 4, 2025 15:29

tianon approved these changes Jul 4, 2025

View reviewed changes

sudo-bmitch approved these changes Jul 4, 2025

View reviewed changes

mtrmac added 3 commits July 15, 2025 15:16

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

d0e1005

…module Signed-off-by: Miloslav Trmač <mitr@redhat.com>

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

cbb69e2

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

Update to github.com/russross/blackfriday/v2

693d8d7

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

mtrmac force-pushed the schemav6 branch from 8384e0f to 693d8d7 Compare July 15, 2025 13:17

tianon approved these changes Jul 17, 2025

View reviewed changes

tianon merged commit ab80ff1 into opencontainers:main Jul 17, 2025
5 checks passed

mtrmac deleted the schemav6 branch July 17, 2025 17:14

mtrmac added a commit to containers/skopeo that referenced this pull request Jul 17, 2025

Merge pull request #2659 from mtrmac/image-spec

7b68a03

Update opencontainers/image-spec after opencontainers/image-spec#1253

sudo-bmitch mentioned this pull request Jul 17, 2025

Fix: Add entry to schema/go.sum #1274

Merged


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

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

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!

mtrmac commented Jul 4, 2025

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

sudo-bmitch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mtrmac commented Mar 14, 2025 •

edited

Loading