snapcraft icon indicating copy to clipboard operation
snapcraft copied to clipboard

chore(legacy-cli): remove unused help and version commands

Open lengau opened this issue 3 months ago • 12 comments

This is a merge of #5731 and #5773, which both implement this change.

Authors: @Serena6688 and @Nalin-Kumar-Gupta

  • [x] Have you followed the guidelines for contributing?
  • [x] Have you signed the CLA?
  • [x] Have you successfully run make lint?
  • [x] Have you successfully run make test?

lengau avatar Sep 16 '25 16:09 lengau

I have raised a PR on top of this to solve the failing integration tests - https://github.com/canonical/snapcraft/pull/5778

Nalin-Kumar-Gupta avatar Sep 17 '25 04:09 Nalin-Kumar-Gupta

@Nalin-Kumar-Gupta the only test failures in this PR are an unrelated bug (https://github.com/canonical/craft-providers/issues/829) and a timeout from a test taking too long, though it was working. What failure are you referring to with your new PR?

bepri avatar Sep 17 '25 12:09 bepri

@Nalin-Kumar-Gupta the only test failures in this PR are an unrelated bug (canonical/craft-providers#829) and a timeout from a test taking too long, though it was working. What failure are you referring to with your new PR?

Thanks for clarifying! To be more precise, my PR addresses the failure that occurs specifically when running snapcraft --version after the legacy help/version commands were removed. Without restoring @click.version_option, snapcraft --version exits with an error instead of printing the version. The other integration test failures you mentioned (craft-providers bug + timeout) are indeed unrelated. My changes don’t affect those.

tests/spread/core24-suites/manifest/manifest-creation/task.yaml (line 28):

["snapcraft_version"]=$(snapcraft --version|cut -d' ' -f2)

Nalin-Kumar-Gupta avatar Sep 17 '25 17:09 Nalin-Kumar-Gupta

I'm not able to reproduce that error. I cloned this PR's branch, packed & installed Snapcraft, and it appears to work:

❯ sudo snap install --dangerous snapcraft_8.12.0.post32_amd64.snap
Please tap your YubiKey.
snapcraft 8.12.0.post32 installed

❯ /snap/bin/snapcraft --version
snapcraft 8.12.0.post32

❯ /snap/bin/snapcraft version
snapcraft 8.12.0.post32

bepri avatar Sep 18 '25 13:09 bepri

I'm not able to reproduce that error. I cloned this PR's branch, packed & installed Snapcraft, and it appears to work:

❯ sudo snap install --dangerous snapcraft_8.12.0.post32_amd64.snap
Please tap your YubiKey.
snapcraft 8.12.0.post32 installed

❯ /snap/bin/snapcraft --version
snapcraft 8.12.0.post32

❯ /snap/bin/snapcraft version
snapcraft 8.12.0.post32

Thanks for checking! I just re-tested and both snapcraft --version and snapcraft version work fine after packing & installing. I must have misunderstood earlier — the failures I mentioned weren’t related to this change. You can merge this I have closed my PR.

Nalin-Kumar-Gupta avatar Sep 18 '25 13:09 Nalin-Kumar-Gupta

@Nalin-Kumar-Gupta it's true that the legacy entrypoint won't have a --version anymore. So if you access the legacy entrypoint, --version will fail, but I haven't managed to come up with any way to access the legacy entrypoint before the new one intercepts --version.

I tried in a directory with a core20 snap, which would be the primary way of entering a legacy entrypoint. If you or @bepri can come up with a way that this snap (which is published for amd64 on the latest/edge/pr-5777 channel) could get there, then we should go with your PR instead (and make a test case for it).

lengau avatar Sep 18 '25 14:09 lengau

@Nalin-Kumar-Gupta it's true that the legacy entrypoint won't have a --version anymore. So if you access the legacy entrypoint, --version will fail, but I haven't managed to come up with any way to access the legacy entrypoint before the new one intercepts --version.

I tried in a directory with a core20 snap, which would be the primary way of entering a legacy entrypoint. If you or @bepri can come up with a way that this snap (which is published for amd64 on the latest/edge/pr-5777 channel) could get there, then we should go with your PR instead (and make a test case for it).

Okay let me see if I can write a test for this. Reopened my PR.

Nalin-Kumar-Gupta avatar Sep 19 '25 16:09 Nalin-Kumar-Gupta

@Nalin-Kumar-Gupta it's true that the legacy entrypoint won't have a --version anymore. So if you access the legacy entrypoint, --version will fail, but I haven't managed to come up with any way to access the legacy entrypoint before the new one intercepts --version.

I tried in a directory with a core20 snap, which would be the primary way of entering a legacy entrypoint. If you or @bepri can come up with a way that this snap (which is published for amd64 on the latest/edge/pr-5777 channel) could get there, then we should go with your PR instead (and make a test case for it).

Hi @lengau The pyproject.toml defines two script entry points:

  • snapcraft = "snapcraft.application:main" (main entry point)
  • snapcraft_legacy = "snapcraft_legacy.cli.__main__:run" (legacy entry point)

While the main snapcraft command intercepts --version before checking for classic fallback, someone could theoretically call snapcraft_legacy --version directly, which would fail if we remove the version functionality from legacy code. I've pushed a test case in the latest commit that demonstrates this edge case here (https://github.com/canonical/snapcraft/pull/5778).

Nalin-Kumar-Gupta avatar Sep 28 '25 15:09 Nalin-Kumar-Gupta

Interesting. I wonder if anybody uses that? The modern snapcraft entrypoint should fall back to the legacy codebase for core20 snap builds anyways, so I don't see why anybody would want to invoke it that way.

Still, I think that probably means we should continue to support it until Snapcraft 9. Thoughts @lengau?

bepri avatar Sep 29 '25 20:09 bepri

Interesting. I wonder if anybody uses that? The modern snapcraft entrypoint should fall back to the legacy codebase for core20 snap builds anyways, so I don't see why anybody would want to invoke it that way.

Still, I think that probably means we should continue to support it until Snapcraft 9. Thoughts @lengau?

Hi @lengau please review

Nalin-Kumar-Gupta avatar Oct 09 '25 13:10 Nalin-Kumar-Gupta

Pretty sure the snapcraft_legacy entrypoint only existed for testing purposes when we migrated that codebase. It's not exposed in the snap (which is the only official way we publish Snapcraft now).

lengau avatar Oct 09 '25 23:10 lengau

I see. I was able to invoke it earlier but I must've been mistakenly in the Python virtual environment for Snapcraft. From a normal install, indeed there is no way to invoke it. We should probably remove that entrypoint.

Thanks for the investigation @Nalin-Kumar-Gupta!

bepri avatar Oct 10 '25 13:10 bepri