Skip to content

Refactor license retrieval to use Conan's Git helper #20447

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 2 commits into
base: main
Choose a base branch
from

Conversation

jellespijker
Copy link
Member

Description

Replaced the external gitpython library with Conan's built-in Git helper for repository operations. Simplified and improved the license retrieval logic while maintaining existing functionality, ensuring better integration and reduced dependency overhead.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Printer definition file(s)
  • Translations

How Has This Been Tested?

  • Locally
  • Test B

Test Configuration:

  • Operating System: Linux Manjaro

Checklist:

Replaced the external `gitpython` library with Conan's built-in `Git` helper for repository operations. Simplified and improved the license retrieval logic while maintaining existing functionality, ensuring better integration and reduced dependency overhead.
Copy link
Contributor

github-actions bot commented Mar 26, 2025

Test Results

23 607 tests  +23 607   23 605 ✅ +23 605   48s ⏱️ +48s
     1 suites +     1        2 💤 +     2 
     1 files   +     1        0 ❌ ±     0 

Results for commit ab44774. ± Comparison against base commit d04360c.

♻️ This comment has been updated with latest results.

@wawanbreton
Copy link
Contributor

I'm afraid many licences descriptions get lost with this change, can you take a look ?

image

@jellespijker jellespijker requested a review from Copilot April 4, 2025 22:09
Copy link

@Copilot 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.

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

Comments suppressed due to low confidence (2)

conanfile.py:219

  • The _format_git_url method is missing a 'self' parameter or should be declared as a static method using the @staticmethod decorator since it is called via self._format_git_url.
def _format_git_url(url):

conanfile.py:258

  • The 'break' statement terminates the candidate tag loop after the first checkout even if no license file is found; consider removing or repositioning it so that subsequent candidate tags can be tried.
break

)

# Clone the repository into a temporary directory.
with tempfile.TemporaryDirectory() as clone_directory:
Copy link
Preview

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

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

Make sure to import the 'tempfile' module at the top of the file to support the usage of tempfile.TemporaryDirectory.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

return file.read()

git.checkout(commit=tag)
except Exception:
Copy link
Preview

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Catching a broad Exception in the git.checkout call may hide important errors; consider catching a more specific exception (or using GitCommandError if applicable) to avoid masking issues.

Suggested change
except Exception:
except GitCommandError:

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

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.

2 participants