Skip to content

[POC] feat: add KPM_NO_SUM env to work without checksum #616

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

suyiiyii
Copy link

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

  • N
  • Y

re #606

2. What is the scope of this PR (e.g. component or file name):

pkg/client/update.go
pkg/client/update_test.go
pkg/env/env.go

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other
  1. add KPM_NO_SUM in env.go
  2. skip get dep checksum when path match KPM_NO_SUM

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other

I tried to complete this feature by referring to the Go implementation.
src/cmd/go/internal/modfetch/sumdb.go:53

You can check the test cases for the matching rules.

I’ve done my best to complete this PR. If there are any shortcomings, please point them out, and I’ll make the necessary changes promptly.

If my solution is approved, please let me know, and I’ll proceed with completing the documentation and other related tasks as soon as possible.

@coveralls
Copy link

coveralls commented Mar 24, 2025

Pull Request Test Coverage Report for Build 15181974607

Details

  • 14 of 19 (73.68%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 42.154%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/client/update.go 14 16 87.5%
pkg/env/env.go 0 3 0.0%
Totals Coverage Status
Change from base Build 15181471869: 0.06%
Covered Lines: 4196
Relevant Lines: 9954

💛 - Coveralls

@zong-zhe zong-zhe closed this May 22, 2025
@suyiiyii
Copy link
Author

Hi @zong-zhe,

Thanks for your ongoing work on maintaining the project. I noticed that this PR was recently closed without any prior feedback or discussion, and I’m a bit confused by the decision.

This PR introduces an experimental feature by adding the KPM_NO_SUM environment variable to allow skipping checksum validation under specific path conditions #606. I tried to follow Go’s internal implementation as a reference (e.g., modfetch/sumdb.go:53) and included test cases for the matching logic.

I understand this may not be a priority or might need further discussion, but I’d really appreciate some feedback on the implementation or the idea itself. If there are issues with the approach or it doesn't align with the project's direction, I’m happy to revise or rework it accordingly.

Please let me know if there's room to reopen this PR, or if you'd prefer to continue the discussion elsewhere.

Thanks again, and looking forward to your thoughts!

@zong-zhe zong-zhe reopened this May 22, 2025
@zong-zhe
Copy link
Contributor

zong-zhe commented May 22, 2025

Hi @suyiiyii 😄

Thanks very much for your contribution. The content corresponding to this PR is not consistent with the issue #606. You may need to adjust the PR content as designed in the issue. We need to refer to the implementation of golang to implement similar functions in kpm.

@suyiiyii
Copy link
Author

Hi @zong-zhe 😄

Thanks for your reply and clarification!

I understand your concern about aligning the PR with the design outlined in issue #606. However, I believe the current implementation is a preliminary attempt toward fulfilling that direction.

In particular, the unit test in this PR (see this file) demonstrates the matching logic for skipping checksum verification when KPM_NO_SUM is set, which aligns with the behavior mentioned in #606 — similar to Go's internal handling in modfetch/sumdb.go.

Of course, I’m happy to refine or restructure the implementation further based on your feedback. If you could point out which parts are missing or need adjustment to better match the intended design, I’ll update the PR accordingly.

This is just an initial implementation, and I’m more than willing to iterate based on your guidance.

Thanks again for your time and support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: kpm add command displaying optimization
3 participants