Skip to content

Conversation

aagor
Copy link
Contributor

@aagor aagor commented Jul 11, 2025

  • Check whole git repository for dirtyness. For the commit hash retrieved, the whole repository is already taken into consideration. This solves the inconsistency between the commit hash retrieval and dirty check.
  • Improve error message.

Changelog: BugFix: Check whole git repository for dirtyness when using revision_mode=scm
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

* Check whole git repository for dirtyness.
  For the commit hash retrieved, the whole repository is already taken into consideration.
  This solves the inconsistency between the commit hash retrieval and dirty check.
* Improve error message.
@CLAassistant
Copy link

CLAassistant commented Jul 11, 2025

CLA assistant check
All committers have signed the CLA.


if git.is_dirty():
raise ConanException("Can't have a dirty repository using revision_mode='scm' and doing"
if git.is_dirty(repository=repository):
Copy link
Member

Choose a reason for hiding this comment

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

I have some concerns that this might be breaking for users that have the repository dirty with some temp files outside of the conanfile subfolder, and suddenly this stops working. Having those temporary files in the root repo wouldn't be a big problem in most cases, isn't it?

Is this check really that important to protect at the repository level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it isn't, at least not for me and not now.
But what is the use-case of using the whole git repo for the ref (as opposed to scm_folder), and having dirty stuff lying around then?

I saw this inconsistency during investigation of different methods for revision_mode and git source retrieval.
If it shouldn't be changed, we should at least add a comment to the source code, that it's the way it currently is on purpose and a link to that PR.

@aagor
Copy link
Contributor Author

aagor commented Jul 14, 2025

I think I have another issue that affects revision_mode for both scm_folder and scm (only if the change above gets not merged).

The dirty check should ensure that the git repository is clean below the folder defined by self.folders.root.
Otherwise you can miss dirty files that are used by Conan, e.g. in this example. E.g. changes in common folder pass the dirty check even if they are clearly relevant.

For revision_mode= "scm_folder", this is also relevant for the commit that gets written into conandata.yml. When something changes only at the common folder, you record the old commit from the hello or bye subfolder, which leads to the fact conan build does checkout something old.

Should I open a separate issue for that?

@aagor
Copy link
Contributor Author

aagor commented Aug 13, 2025

Hi @memsharded,

do you have any feedback on my questions above?
Without it, I can't proceed and adapt this PR if necessary.
Thanks.

@aagor
Copy link
Contributor Author

aagor commented Sep 3, 2025

@memsharded Ping.

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.

3 participants