-
Notifications
You must be signed in to change notification settings - Fork 21
feat: publish the cli as snap #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: karan-palan <[email protected]>
Signed-off-by: karan-palan <[email protected]>
.github/workflows/snap.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extend the test
workflow with a new snap
entry that builds the Snap (but doesn't publish it) so that we make sure to never break the integration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can you add a new snap
job on the package.yml
action that will build the snap and build it as an artefact, and then extend the existing publish
job to download the snap artefact and publish it (if it is a tag release)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file can go away then?
…allback to 0.00 version if no tags provided - Update snapcraft.yaml for building the JSON Schema CLI as a snap - Update GitHub Actions workflow for building and publishing snaps - Update CI to include snap building and testing - Add snap artifacts to .gitignore Signed-off-by: karan-palan <[email protected]>
Signed-off-by: karan-palan <[email protected]>
Signed-off-by: karan-palan <[email protected]>
Signed-off-by: karan-palan <[email protected]>
…ema into feat/publish-as-snap
…er to build snap sucessfully Signed-off-by: karan-palan <[email protected]>
…er to build snap sucessfully Signed-off-by: karan-palan <[email protected]>
snapcraft.yaml
Outdated
|
||
override-pull: | | ||
craftctl default | ||
VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo "v0.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should fetch the version from CMakeLists.txt
.
snapcraft.yaml
Outdated
-DCMAKE_SHARED_LIBRARY_LINK_C_FLAGS="-Wl,-Bstatic" \ | ||
-DCMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS="-Wl,-Bstatic" \ | ||
$CRAFT_PART_SRC | ||
cmake --build build --parallel "$(nproc)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake --build build --parallel "$(nproc)" | |
cmake --build build --parallel "$(nproc)" --config Release |
snapcraft.yaml
Outdated
-DCMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS="-Wl,-Bstatic" \ | ||
$CRAFT_PART_SRC | ||
cmake --build build --parallel "$(nproc)" | ||
install -Dm755 build/src/jsonschema "$CRAFT_PART_INSTALL/bin/jsonschema" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be using cmake --install
rather than manually pulling the binary out with install
snapcraft.yaml
Outdated
override-build: | | ||
cmake -B build \ | ||
-DCMAKE_BUILD_TYPE=Release \ | ||
-DCMAKE_INSTALL_PREFIX=/usr \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be $CRAFT_PART_INSTALL
given what you had below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is invalid. You should only pass the prefix when calling cmake --install
below
snapcraft.yaml
Outdated
|
||
override-build: | | ||
cmake -B build \ | ||
-DCMAKE_BUILD_TYPE=Release \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DCMAKE_BUILD_TYPE=Release \ | |
-DCMAKE_BUILD_TYPE:STRING=Release \ |
snapcraft.yaml
Outdated
cmake -B build \ | ||
-DCMAKE_BUILD_TYPE=Release \ | ||
-DCMAKE_INSTALL_PREFIX=/usr \ | ||
-DBUILD_SHARED_LIBS=OFF \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DBUILD_SHARED_LIBS=OFF \ | |
-DBUILD_SHARED_LIBS:BOOL=OFF \ |
snapcraft.yaml
Outdated
|
||
summary: Command-line tool for working with JSON Schema | ||
description: | | ||
The JSON Schema CLI is a comprehensive command-line utility for validating, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use the exact same text as the GitHub repo description?
The CLI for working with JSON Schema. Covers formatting, linting, testing, bundling, and more for both local development and CI/CD pipelines
.github/workflows/test.yml
Outdated
uses: actions/upload-artifact@v4 | ||
with: | ||
name: snap-${{ github.sha }}.snap | ||
path: ${{ steps.snapcraft.outputs.snap }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also manually install the .snap
file and run i.e. jsonschema version
just to confirm it runs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or even better, assuming jsonschema
will end up in the path, try to run one or two representative test cases from ./test
, like:
./test/ci/pass_validate_http.sh jsonschema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try this myself in the mean-time. I'll push a commit
.gitignore
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire project build happens in ./build
to avoid polluting the project and having to ignore things out. Can you perform the build in ./build/snap
? Not sure if snapcraft
lets you put an output directory, but otherwise you can cd ./build/snap
and call snap craft from there
51a83ce
to
a4665b3
Compare
.github/workflows/package.yml
Outdated
merge-multiple: true | ||
|
||
# Generate signed checksums for all pre-built binaries | ||
# Generate signed checksums for all pre-built binaries and snaps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snaps are prebuilt binaries anyway!
.github/workflows/package.yml
Outdated
@@ -167,6 +187,17 @@ jobs: | |||
with: | |||
node-version: '22.x' | |||
registry-url: 'https://registry.npmjs.org' | |||
- name: Publish Snap to Snap Store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose we first publish the snaps to GitHub Releases. Once we make sure we got that right and that it all works well, we can look into publishing them to the official store. That way we can test all of this a few times before hitting the store for real
4abab45
to
c140b3a
Compare
Signed-off-by: Juan Cruz Viotti <[email protected]>
c140b3a
to
c190184
Compare
I added a commit on top of yours making a few adjustments and simplifying things a bit. The 3 follow-ups are:
|
See: #369 Signed-off-by: Juan Cruz Viotti <[email protected]>
See #371, in case you have any idea of what might be going off |
And this is the other one that I want to try to fix: #372 |
See: #369 Signed-off-by: Juan Cruz Viotti <[email protected]>
See: #369 Signed-off-by: Juan Cruz Viotti <[email protected]>
See: #369 Signed-off-by: Juan Cruz Viotti <[email protected]>
See: #369 Signed-off-by: Juan Cruz Viotti <[email protected]>
OK, I think I fixed everything. I'm trying the first GitHub Release deployment and we'll see how that goes |
See https://github.com/sourcemeta/jsonschema/releases/tag/v9.4.0 for an initial GitHub Release. Thanks a lot for this! Let's give it a shot this week and if it all works fine, we'll publish to the store |
That's great news! I saw the changes you made, and everything's working perfectly now. It's much more simplified, which is a huge improvement. I wish I could've helped out, but I was asleep at the time. Anyway, I'm really looking forward to the CLI being available as a Snap and getting those automatic updates! |
For linux based operating systems, the cli must be published as a snap on the snap store for automatic updates
This PR adds Snap packaging and automatons:
• Uses cmake plugin to build the C++ CLI on core22.
• Dynamically adopts the version from the most-recent Git tag so each new tag pushes a fresh Snap revision.
• Exposes a strict-confinement
jsonschema
app with home and network plugs.
• Builds the snap on every version tag (v*..) or manual dispatch.
• Stores the artifact and, when a tag build, publishes it to the Snap Store using a secret SNAPCRAFT_STORE_CREDENTIALS.