Skip to content

fix: resolve set() shadowing and sparse checkout ref issues#184

Merged
danielmeppiel merged 2 commits intomicrosoft:mainfrom
Jah-yee:fix/cli-set-builtin-sparse-checkout
Mar 6, 2026
Merged

fix: resolve set() shadowing and sparse checkout ref issues#184
danielmeppiel merged 2 commits intomicrosoft:mainfrom
Jah-yee:fix/cli-set-builtin-sparse-checkout

Conversation

@Jah-yee
Copy link
Copy Markdown
Contributor

@Jah-yee Jah-yee commented Mar 6, 2026

Summary

This PR fixes two bugs reported in issue #179:

Bug 1: set() builtin shadowed by Click command

  • File: src/apm_cli/cli.py
  • Problem: set(_pre_download_results.keys()) invokes Click's config set command instead of Python's builtin set() because @config.command decorator defines def set(key, value) which overwrites the set = builtins.set alias.
  • Fix: Use builtins.set(_pre_download_results.keys()) explicitly.

Bug 2: sparse checkout fetches wrong ref

  • File: src/apm_cli/deps/github_downloader.py
  • Problem: When no ref is provided, git fetch origin --depth=1 fetches all branch tips and FETCH_HEAD points to the wrong commit, causing the expected subdirectory to be missing after checkout.
  • Fix: Always pass an explicit ref to git fetch - defaults to HEAD if not provided.

Changes

  • cli.py: Changed set(...) to builtins.set(...) (line 1844)
  • github_downloader.py: Changed conditional ref append to always pass ref (default to 'HEAD') (line 1076)

Testing

  • Syntax validation passed (python3 -m py_compile)
  • These fixes address the Release Validation CI job failure

Fixes #179

- cli.py: Use builtins.set() instead of set() to avoid Click command shadowing
- github_downloader.py: Always pass explicit ref to git fetch (default to HEAD)

Fixes microsoft#179
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes two reported regressions: set() being shadowed by a Click command in the CLI, and sparse checkout occasionally checking out the wrong commit by fetching an ambiguous ref.

Changes:

  • Use builtins.set(...) explicitly to avoid calling the Click set command handler.
  • Ensure sparse checkout always fetches an explicit ref (defaulting to HEAD) before checking out FETCH_HEAD.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/apm_cli/deps/github_downloader.py Adjusts git fetch invocation to always include a ref for sparse checkout flows.
src/apm_cli/cli.py Avoids set() shadowing by explicitly referencing builtins.set().

Comment thread src/apm_cli/deps/github_downloader.py
Comment thread src/apm_cli/deps/github_downloader.py Outdated
@danielmeppiel danielmeppiel merged commit e9d5ba1 into microsoft:main Mar 6, 2026
5 of 6 checks passed
@Jah-yee Jah-yee deleted the fix/cli-set-builtin-sparse-checkout branch March 6, 2026 10:56
danielmeppiel added a commit that referenced this pull request Apr 30, 2026
…#1073)

* docs(notice): rename NOTICE.md -> NOTICE; add CLA third-party section

Two changes, one file rename:

1. Rename NOTICE.md -> NOTICE, matching the Apache / CNCF convention used
   by upstream third-party-attribution files (kubernetes-sigs/kro,
   kubernetes-sigs/headlamp, etc.). The .md extension was non-idiomatic
   for a generated legal artifact -- NOTICE files are read by tooling
   (license scanners, SBOM generators) that match on the bare filename.
   Generator (scripts/generate-notice.py), Makefile target, and the
   NOTICE Drift Check workflow are all updated to operate on the
   extension-less path.

2. Add a 'Submitted on behalf of a third-party' section to NOTICE,
   crediting five contributors whose pull requests landed before the
   microsoft-github-policy-service CLA bot recorded a signature on
   file. The repo transferred from danielmeppiel/awd-cli to the
   microsoft org; some early PRs predate CLA enforcement, and we
   could not retroactively reach all contributors. Mirrors section 7
   of common CLA texts (the wording adopted by CNCF NOTICE files).

   Driven by a new _third_party_submissions block in
   scripts/notice-metadata.yaml -- legally-significant wording stays
   alongside the per-component data, not buried in code.

   Contributors named (verified via Check Runs API against the
   microsoft-github-policy-service app, license/cla check on every
   merged PR by each suspected author):
     - @pofallon  (PR #4)
     - @richgo    (PRs #8, #25, #26, #33, #34)
     - @ryanfk    (PR #92 -- bot ran with conclusion=null,
                  output: 'Contributor License Agreement is not agreed yet.')
     - @foutoucour (PR #108)
     - @Jah-yee   (PR #184)

   Listed contributors who later sign the CLA (or who were signed
   under a different GitHub account at the time) can request removal
   via issue.

Co-authored-by: Copilot <[email protected]>

* docs(notice): trim third-party section preamble

Strip the historical/CNCF-citation paragraph and the verbatim CLA-section-7
quote. Keep only the active sentence (what the listing means + how to
request removal).

Co-authored-by: Copilot <[email protected]>

* docs(notice): address PR #1073 review

Three fixes from copilot-pull-request-reviewer:

1. Drop spurious leading '---' separator in the third-party-submissions
   renderer. render_component already ends each component with '---\n\n',
   so prepending another '---' produced two consecutive separators in
   NOTICE. Verified: separator count dropped from 17 to 16.

2. Sweep stale 'NOTICE.md' references in scripts/generate-notice.py
   (top-level docstring, Modes section, ComponentMeta and DepSpec field
   docstrings). The constant was renamed; the docs lagged.

3. Append (#1073) PR refs to both CHANGELOG entries; ASCII-correct the
   arrow ('->' instead of '->').

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.

[BUG] set() builtin shadowed by Click command + sparse checkout fetches wrong ref

3 participants