Skip to content

Fix GitLab clone URLs and authentication in pr_diff Dockerfiles - #94

Merged
adithya-s-k merged 1 commit into
huggingface:mainfrom
abhinavgautam01:fix/93-pr-diff-gitlab-clone-url
Sep 15, 2026
Merged

adithya-s-k merged 1 commit into
huggingface:mainfrom
abhinavgautam01:fix/93-pr-diff-gitlab-clone-url

Conversation

@abhinavgautam01

Copy link
Copy Markdown
Contributor

Fixes #93

Summary

GitLab merge requests were mined through the correct provider, but their emitted pr_diff Dockerfiles cloned a hardcoded GitHub URL. This could fail the environment build or target a different repository.

Build clone URLs from the source repository URL, preserving the host and full path. Normalize supported SSH-style inputs to HTTPS and select GITHUB_TOKEN or GITLAB_TOKEN using the existing authentication helper.

Quote token expansion, remove credentials supplied in source URLs and retain the clean origin reset after cloning. Update documentation to describe provider-specific authentication.

Validation

Add 15 regression cases covering GitHub compatibility, GitLab URLs, nested paths, SSH-style inputs, credential stripping and public/authenticated clone commands.

The shell tests execute the generated clone commands with a fake Git executable and dummy tokens, checking argument handling and origin cleanup without network access.

  • Python 3.12, 3.13 and 3.14: 731 passed, 5 skipped on each.
  • Ruff lint, formatting and whitespace checks passed.
  • Source distribution and wheel builds passed.
  • Clean wheel installation with locked dependencies, CLI version check and dependency compatibility check passed.

Scope

Private GitLab MR diff fetching remains unsupported under #65. This change fixes emitted clone URLs and build-time authentication; it does not establish end-to-end private GitLab mining support.

@KNambiarDJsc KNambiarDJsc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed and tested 719238d:

  • CI-equivalent run on Linux: ruff is clean, and the full suite passes on py3.12 and py3.14 (730 passed).
  • Generated Dockerfiles, main vs this PR:
    • --repo pallets/click: the only change is the quoted token expansion, x-access-token:"${GITHUB_TOKEN}"@github.com/.... The shell treats it the same, so GitHub tasks are unaffected.
    • --repo https://gitlab.com/python-devs/importlib_resources: main emitted ARG GITHUB_TOKEN and cloned https://github.com/python-devs/importlib_resources.git. With this PR it emits ARG GITLAB_TOKEN, clones through oauth2:"${GITLAB_TOKEN}"@gitlab.com/..., and uses the correct clean URL.
  • The quoting tests are thorough. They run the real RUN body through /bin/sh with a fake git, including a token containing spaces, *, $HOME and $(false).

LGTM. Two non-blocking follow-ups:

  1. docs/reference/AUTH.md is still GitHub-only. Its "Private repos at task build time" section says the Dockerfile declares ARG GITHUB_TOKEN= and clones through x-access-token:<token>@github.com/..., so a consumer following it for a GitLab task would pass the wrong build arg. Mentioning GITLAB_TOKEN and oauth2: there would match the updated pr_diff.md.
  2. Windows test runs. The 6 test_clone_shell_uses_correct_token_and_scrubs_origin cases fail on Windows with FileNotFoundError, because there's no /bin/sh. CI is Linux-only, so there's no CI impact. Adding pytest.mark.skipif(not Path("/bin/sh").exists(), reason="needs a POSIX shell") would keep the suite usable there; #108 tracks the other Windows issues.

@adithya-s-k
adithya-s-k merged commit aeee03d into huggingface:main Sep 15, 2026
5 checks passed
@adithya-s-k

Copy link
Copy Markdown
Collaborator

thanks @abhinavgautam01, merged! appreciate the GitLab clone and auth fixes, especially the coverage around credentials.

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.

Use the source repository host when emitting pr_diff Dockerfiles for GitLab

3 participants