NotificationsYou must be signed in to change notification settings
Fork825
Star11k

refactor: consistent cloning & pattern-handling#388

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

filipchristiansen wants to merge9 commits intomain

base:main

Choose a base branch

fromresolve-commit

Open

refactor: consistent cloning & pattern-handling#388

filipchristiansen wants to merge9 commits intomainfromresolve-commit

Conversation

Copy link

Contributor

filipchristiansen commentedJul 6, 2025•
edited
Loading

✨ Refactor: consistent cloning & pattern-handling

Closes#196
Closes#344
Prepares for#342 (digest prefix/suffix) and#343 (caching)

Why

Cloning behaved differently depending on whether they supplied a branch, tag or commit.
→Caching and reproducibility suffered.
Pattern handling lived inquery_parser and duplicated ignore/include logic.
→Hard to unit-test and reuse.
Windows CI occasionally failed when temp clones contained read-only
packed-object files.
→Shallow-clone cleanup broke with WinError 5.

What’s new

Unified commit resolution
utils.git_utils.resolve_commit() guarantees we always fetch the exact SHA
(HEAD / branch / tag)before checkout.
Deterministic → enables caching (feat: Implement caching on a per-commit basis #343).
New pattern helpers
utils.pattern_utils.process_patterns() centralises include/exclude parsing
(moved out ofquery_parser). Adds thorough tests.
Windows-safe repo cleanup
_handle_remove_readonly on-error callback makes read-only Git objects
writable and retriesshutil.rmtree(), preventing WinError 5 in CI.
Public API cleanup (BREAKING)
- parse_query()removed in favour of
  - parse_remote_repo() (URLs/slugs)
  - parse_local_dir_path() (local paths)
- clone_repo,ingest_query,parse_query are no longer re-exported from
  gitingest.__init__.
Clone rewrite
- Single entry point that always does: shallow clone → sparse-checkout
  (optional) → fetch commit → checkout → submodule update (optional).
- _checkout_partial_clone renamed & moved →git_utils.checkout_partial_clone.
Server updates
- query_processor uses new pattern utilities and passes a typed enum.
- IngestRequest.validate_input_text() now removes any.git suffix.
Delete_is_safe_symlink
Remove pattern validation (delete_is_valid_pattern,InvalidPatternError, andtest_parse_patterns_invalid_characters)
Tests added/updated
clone, pattern, parser, summary

File changes

`gitingest`

File	Changes
`__init__.py`	Stop exposing internal helpers (`clone_repo`,`ingest_query`,`parse_query`)
`clone.py`	Rewrite`clone_repo` to call`resolve_commit`, sparse checkout, and uniform fetch/checkout steps; move helper & rename
`entrypoint.py`	Add`_handle_remove_readonly` callback for Windows temp-dir cleanup;`ingest_async` now uses`parse_remote_repo` /`parse_local_dir_path`
`output_formatter.py`	Add missing`Tag` prefix in`_create_summary_prefix`
`query_parser.py`	Remove`parse_query`; move pattern helpers to`pattern_utils.py`
`utils/exceptions.py`	Delete`InvalidPatternError`
`utils/git_utils.py`	New`resolve_commit` helper
`utils/os_utils.py`	Rename`ensure_directory` →`ensure_directory_exists_or_create`
`utils/pattern_utils.py`	Consolidated helpers +`process_patterns`
`utils/query_parser_utils.py`	Delete`_is_valid_pattern`
`path_utils.py`(removed)	Delete obsolete helper`_is_safe_symlink`

`server`

File	Changes
`models.py`	Strip`.git` suffix in`validate_input_text`
`query_processor.py`	Replace`parse_query` with`parse_remote_repo`, integrate`PatternType`, use`process_patterns`
`routers_utils.py`	Coerce`pattern_type` into`PatternType` enum

`tests`

File	Changes
`conftest.py`	Adjust fixtures
`query_parser/test_git_host_agnostic.py`	Switch to`parse_remote_repo`
`query_parser/test_query_parser.py`	Switch to`parse_remote_repo`; add`parse_local_dir_path` coverage
`test_clone.py`	Update expectations and fixtures
`test_pattern_utils.py`	Add tests for`_parse_patterns` and`process_patterns`; remove`test_parse_patterns_invalid_characters`(pattern validation no longer enforced)
`test_summary.py`	Verify that`gitingest.ingest()` emits correct summaries

filipchristiansen mentioned this pull request

Jul 6, 2025

feat: avoid using branch references#344

Open

filipchristiansen changed the title~~resolve commit~~feat: resolve commit

Jul 9, 2025

github-advanced-securitybot found potential problems

Jul 9, 2025

View reviewed changes

.github/workflows/pr-title-check.yml FixedShow fixedHide fixed

src/server/routers/ingest.py FixedShow fixedHide fixed

github-advanced-securitybot found potential problems

Jul 9, 2025

View reviewed changes

src/static/js/utils.js FixedShow fixedHide fixed

filipchristiansen force-pushed theresolve-commit branch 3 times, most recently from7ab4af2 to831a36dCompare

July 9, 2025 20:32

filipchristiansen requested a review fromCopilot

July 9, 2025 20:33

This comment was marked as resolved.

filipchristiansen force-pushed theresolve-commit branch 6 times, most recently fromcf1aa6f tod1224a6Compare

July 9, 2025 23:45

filipchristiansen added the enhancement label

Jul 9, 2025

filipchristiansen marked this pull request as ready for review

July 9, 2025 23:47

filipchristiansen requested review fromCopilot andNicolasIRAGNE

July 9, 2025 23:48

This comment was marked as resolved.

filipchristiansen force-pushed theresolve-commit branch 2 times, most recently from979c88a to3031e7cCompare

July 10, 2025 00:07

filipchristiansen changed the title~~feat: resolve commit~~refactor: consistent cloning & pattern-handling

Jul 10, 2025

filipchristiansen requested a review fromix-56h

July 12, 2025 19:39

ix-56h reviewed

Jul 12, 2025

View reviewed changes

src/gitingest/entrypoint.pyShow resolvedHide resolved

ix-56h reviewed

Jul 12, 2025

View reviewed changes

.github/workflows/publish_to_pypi.yml OutdatedShow resolvedHide resolved

ix-56h reviewed

Jul 13, 2025

View reviewed changes

src/gitingest/entrypoint.pyShow resolvedHide resolved

This comment was marked as resolved.

ix-56h approved these changes

Jul 13, 2025

View reviewed changes

Copy link

Contributor

ix-56h left a comment

Choose a reason for hiding this comment

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

LGTM

This comment was marked as off-topic.

NicolasIRAGNE reviewed

Jul 14, 2025

View reviewed changes

src/gitingest/clone.py

		if not await check_repo_exists(url, token=token):
		msg = "Repository not found. Make sure it is public or that you have provided a valid token."
		raise ValueError(msg)

		commit = await resolve_commit(config, token=token)

Copy link

Contributor

NicolasIRAGNEJul 14, 2025

Choose a reason for hiding this comment

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

Why not populate the CloneConfig directly?

Copy link

Contributor

NicolasIRAGNEJul 14, 2025

Choose a reason for hiding this comment

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

My logic was that we should be able to reuse that after when manipulating the object

NicolasIRAGNE reviewed

Jul 14, 2025

View reviewed changes

src/gitingest/utils/pattern_utils.py OutdatedShow resolvedHide resolved

This comment was marked as duplicate.

Copy link

ContributorAuthor

filipchristiansen commentedJul 14, 2025

TODO:

delete_is_valid_pattern,InvalidPatternError,_is_safe_symlink
callresolve_commit inparse_remote_repo instead of inclone_repo

filipchristiansenand others added9 commits

July 16, 2025 21:16

resolve commit

fe004b7

add test_summary.py

d7601f6

replace non-ASCII chars

93ee16e

fix(entrypoint): handle read-only Git objects on Windows

abb7cf5

Add `_handle_remove_readonly` on-error callback for `shutil.rmtree`that removes the read-only attribute and retries, preventing WinError 5during temp-repo cleanup in tests.

python versions compat: replace onerror with onexc for Python 3.12+

44cad26

fix typo

6469354

add more blob/tag tests

541b310

fix blob/tag added test

chore: delete _is_valid_pattern, InvalidPatternError, and _is_safe_sy…

3af9b60

…mlink

chore: delete test_parse_patterns_invalid_characters

8438492

filipchristiansen force-pushed theresolve-commit branch fromf98250f to8438492Compare

July 16, 2025 19:16

Labels

enhancement

3 participants

Movatterモバイル変換

refactor: consistent cloning & pattern-handling#388

Are you sure you want to change the base?

refactor: consistent cloning & pattern-handling#388

Uh oh!

Conversation

filipchristiansen commentedJul 6, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✨ Refactor: consistent cloning & pattern-handling

Why

What’s new

File changes

gitingest

server

tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

ix-56h left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

NicolasIRAGNEJul 14, 2025

Choose a reason for hiding this comment

Uh oh!

NicolasIRAGNEJul 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as duplicate.

filipchristiansen commentedJul 14, 2025

TODO:

Uh oh!

Uh oh!

filipchristiansen commentedJul 6, 2025•
edited
Loading

`gitingest`

`server`

`tests`