Support optional conf-based arguments for controlling dirty repo detection

[santhonisz]

What is your suggestion?

Hi there,

Under Conan 2.0 we have been coming against an issue when attempting to use the scm and scm_folder revision modes on our private Conan project recipes. To cut a long story short, we are often required to have a particular file located alongside the conanfile.py; this file is both tracked by git (it cannot be ignored as it must be stored in the repo) and is modified during our CI build processes prior to the Conan package build. As such, when we attempt to either conan create or conan export etc. we receive the error about the repo being dirty and the command fails.

I should point out that this file in no way contributes to the build outputs of the Conan package, so it being dirty has no effect on the built package.

I realise that #14330 has gone some of the way to allowing dirty content to exist when using scm_folder revisions, but in our specific case it still doesn't provide a solution due to the modified file being located in the same folder as the recipe. It is simply not viable for us to re-organise the structure of our repos so that we could use the functionality in this form.

Would a possible solution to this scenario be to provide an optional config setting that could allow users to provide extra arguments to the git status -s command used in the revision calculcation logic, so that specific files/patterns can be excluded from the dirty check e.g. tools.git:is_dirty_args=':!<file_to_exclude>' (see https://stackoverflow.com/a/67815271)

It would also be good to have the ability to pass such extra arguments via an optional arg for the is_dirty method, and the get_url_and_commit method that calls it, within the conan.tools.scm.Git tool.

I look forward to your feedback!

Have you read the CONTRIBUTING guide?

• I've read the CONTRIBUTING guide

[memsharded]

Hi @santhonisz

Thanks for your suggestion. I understand the use case, and it seems enough to be considered.

If this is like something permanent, and you point that you want that in the recipe too for the Git() helper, wouldn't it make sense to have it as a recipe attribute so it applies always to both things, and not require a conf? Like scm_excludes = ...? (just thinking out loud, and trying to understand the angles).

[santhonisz]

Hi @memsharded ,

Thanks for the prompt reply.

Actually your suggestion probably makes more sense. We are using a custom base class for the majority of our Conan recipes where we follow the approach outlined in Capturing git scm information, so having a recipe attribute that was automatically considered would make it very easy for us to apply it broadly.

Thinking out loud myself, I wonder if rather than just configuring exclusions, there would be any appetite for an scm_sources attribute that behaved like the export_sources attribute; allowing both inclusion and exclusion patterns to be set. The dirty repo detection logic could then apply those if the attribute was present on the recipe, or continue to use the current logic if it is not? We don't need the functionality to be this advanced ourselves; the scm_excludes you proposed would satisfy out requirements, just trying to think more broadly.

[memsharded]

After comments from #14715 (comment), adding this for next 2.0.12 discussion and consideration.

[paorho]

Hi everybody,
we have a similar use case:
into ours CI we have the need to build the same artifacts with both conan v1 and conan v2; the CI process add files and modify template files to add build info.

The better solution for our use case is to add an option to export-pkg command and eventually other commands involved to allow scm revision_mode even on dirty repository, like conan v1 does.

We use Gitlab as source repository and CI manager and Artifactory to store the result.
The CI processes c++, java and shell projects, using a lot of bash and python scripts.
For the c++ projects the conan v1 commands flow still used:
conan install, conan build, conan export-pkg, conan upload,
implemented in different scripts to perform the prepare, check, build and finalize steps of the CI;
migrate to conan create command it's hard to think.
The CI process collects build info sent by e-mail and inserted into the built artifact, postgresql db and Confluence pages.

In this scenario an option to add to the conan commands is easy to implement into the scripts.

The expected result in Artifactory is, like conan v1 commands do:
https://xyz.artifactory.it/artifactory/webapp/#/artifacts/browse/tree/General/group/subgroup/project/version/branch/fd98bd4e834e7400133f21f44e42c7c2e4d9cf70
where fd98bd4e834e7400133f21f44e42c7c2e4d9cf70 is the project's commit hash pushed to GitLab generating the CI pipeline.

[memsharded]

@santhonisz, a couple of clarifications, please

our private Conan project recipes. To cut a long story short, we are often required to have a particular file located alongside the conanfile.py; this file is both tracked by git (it cannot be ignored as it must be stored in the repo) and is modified during our CI build processes prior to the Conan package build. As such, when we attempt to either conan create or conan export etc. we receive the error about the repo being dirty and the command fails.

I should point out that this file in no way contributes to the build outputs of the Conan package, so it being dirty has no effect on the built package.

This file lives in the user folder, together with the conanfile.py, but is it exported to the Conan cache as part of the recipe? Or it only exist in the user folder in pre-export?

In think that for files that are not exported, then, it might not be necessary to do exclusion mechanism, but just taking the git commit, and using it as if it were not dirty? Or you are interested in the protection of the "dirty" checking for other potential files?

If the file is actually exported into the cache, that might be more problematic, because even if it is not used (why does it need to be exported then), it will break the "immutability" of package recipes. Maybe this would be a candidate for the new metadata files feature? (merged, to be documented and launched soon).

[santhonisz]

Hi @memsharded ,

The file in question is not exported to the Conan cache; it is in fact a sonar-project.properties file that configures the SonarQube analysis of the package sources. As previously mentioned, this file is committed to the repo and is then dynamically updated as part of our CI build process, hence making the repo "dirty".

It's an interesting point you make about just taking the git commit, and using it as if it were not dirty. While I think that would be an acceptable solution for us in terms of CI builds, I do like the protection the current "dirty" checking provides for developers who are running conan create/export locally e.g. if they have uncommitted source changes in their user folder, bypassing the dirty check would lead to inconsistent results when running conan create/export vs conan build.

To your last point, I completely agree and we have no such scenario where a "dirty" file is exported because, as you say, that would break the "immutability" of the package.

[BriceMortier]

Hello,

Protecting from dirty sources and keeping immutability makes total sense. Nevertheless, isn't there a bug in the is_dirty method? It is mentionned that the current folder is checked, but the executed Git command is git status -s. This will check the whole repository for dirty files. The command is missing the folder option: git status -s -- ".". As a consequnce, it is currently not possible to call get_url_and_commit as soon as there is a modification in the repository, even in a different folder than the recipe.

[memsharded]

Nevertheless, isn't there a bug in the is_dirty method? It is mentionned that the current folder is checked, but the executed Git command is git status -s. This will check the whole repository for dirty files. The command is missing the folder option: git status -s -- ".". As a consequnce, it is currently not possible to call get_url_and_commit as soon as there is a modification in the repository, even in a different folder than the recipe.

Thanks for pointing this out. Yes, I think it is worth checking this, lets have a look

[memsharded]

I am doing #15289 for fixing the bug of Git.is_dirty(), sorry for the delays, it has been quite busy, I am trying to revive this feature request, but the bugfix has higher priority.

[memsharded]

By the way, I am checking possible implementations, and it seems git already has some mechanisms to define this like:

# do not track changes
git update-index --assume-unchanged path/to/file

# continue tracking changes
git update-index --no-assume-unchanged path/to/file

At least that is a potential workaround that CI can define to avoid the Conan recipe failing because of dirty?

[santhonisz]

No worries on the delays, appreciate there's been more pressing issues!

To your comment, we've in fact been doing something similar in CI to date, though with --skip-worktree/--no-skip-worktree in place of --assume-unchanged/--no-assume-unchanged. It is workable, though we'd obviously like to see a cleaner/less-impactful solution in place such as passing the aforementioned exclusion patterns to the git status command.

[memsharded]

I am trying to find a way to have git be the one that executes the exclusion. Because we did it in the past, and it was not that evident, a bit too complicated and fragile.

I am thinking about trying to add something to .gitignore temporarily or some other similar git trick, like --assume-unchanged, passing the user excluded argument, but without having to filter ourselves the changeset.

[memsharded]

#15289 is doing the fix for the path is_dirty() for 2.0.15, will try the exclude for next release

[memsharded]

I have tried several git built-in approaches, but I didn't find anything effective

I am trying in #15457 a git_excluded = ["myfile*.txt", ...] fnmatch based approach, that works both for tracked and untracked files modifications. Feedback and testing very welcome.

[memsharded]

We are moving forward #15457 for next 2.1.
It was changed to Git(..., excluded=[...]) and revision_mode_excluded = [...] for the revision_mode = "scm" case

It would be fantastic to have your feedback on this before it goes live.

[paorho]

Hi memsharded,
unfortunately this solution does not meet our needs.
Editing each project's conanfile.py file every time the CI process changes to add a new file or edit a new template to export is not an option.
The need is a way to use the git commit hash.
The best way is an option of the conan export-pkg command: only the CI procedures need to be modified.
The other way is a new revision_mode: the conanfile.py of all projects only needs to be modified once.
In any case, no checks or calculations are necessary, just get the git commit hash.

[memsharded]

Thanks for the feedback @paorho

Isn't it what you are looking for the revision_mode_excluded = ["*"]? (if we are talking about revision_mode = "scm", is this the case?

This exclusion is just a protection for users to avoid having necessary changes not committed, and allow to proceed with the commit hash. If the files are going to change, then maybe the above makes sense?

[paorho]

I hadn't considered using wild cards.

Then adding the line
revision_mode_excluded = ["*"]
where
revision_mode = "scm"
is defined should be sufficient to achieve the desired effect,
correct?

[memsharded]

Yes, exactly it should.

The only thing you will not get is the protection against doing unwanted changes to source files that would be part of the package, but that should be relatively low risk.
I am having a look to see if it is possible to have something more conf driven from the outside, but it is a bit challenging, as export operation do not receive profiles.

[paorho]

OK

[memsharded]

For other users use cases: the in-recipe explicit Git() in export() method might be directly configurable for CI only with env-vars, something like:

 def export(self):
       excluded = os.getenv("MY_GIT_EXCLUDED")
       excluded = excluded.split(",") if excluded else None
       git = Git(self, self.recipe_folder, excluded=excluded)
       git....

As said above, command line or profile --conf wouldn't be possible at this moment because the export operation do not use profile information, but this could be a reasonable approach.

I will have a look if global.conf would work for this

[memsharded]

I have made it configurable in same PR via global.conf with core.scm:excluded=[] (list of patterns), please retry @paorho

[paorho]

Hi memsharded,
after installing from source of the development2 branch,
I tried git config --global --add core.scm.excluded [*],
due to git config --global --add core.scm:excluded [*] returns invalid key error,
but it does not work.

Do I have to do something else?

[memsharded]

Hi @paorho

Those are not git configs, but Conan configs, you can use them:

• put them in your global.conf file in the Conan home folder
• We are working to make them also available in teh command line as conan command --core-conf=...., it will most likely be in Conan 2.1 (but at the moment you can verify by adding things in your global.conf file)

In order to test from source:

• You need to checkout or use the branch of the PR, not develop2 branch
• The correct config is core.scm:excluded, please note the ":"
• Put it in global.conf in your cache. The right value would be more like ["*"], quoting the value

[paorho]

Hi memsharded,
how can I do this:
'checkout or use the branch of the PR'?
Have I to clone some specific repo?
I follwed the instruction in the 'Install from source' section of 2.0.17 doc

[memsharded]

Something like

git clone https://github.com/memsharded/conan conan_src
cd conan_src
git checkout feature/git_excluded
pip install -e .

And that will be running Conan from my branch. Recommended to do in a python virtualenv, that you can later destroy

[paorho]

Hi memsharded,
it works.
Thanks for the functionality, information, suggestions and support.
I use docker containers for testing.

[memsharded]

Excellent! Thanks very much for the feedback, I'll keep pushing it for 2.1

@santhonisz @BriceMortier any further feedback? Thanks!

[BriceMortier]

On my side I upgraded to 2.0.15 to use the fix on the is_dirty method and it works fine. Thank you.

[memsharded]

Merged in #15457, this will be in next 2.1, thanks all for the feedback