Warn when using patching function and no_copy_source is True

[AbrilRBS]

Changelog: Feature: Warn when patching files and the recipe has no_copy_source = True, which could lead to unforseen issues
Docs: Omit

Some questions remain as to:

• Should we check that we're trying to modify something from the build() method and not the package/source one?
• Should we check that we're modifying something inside self.source_folder?
• Anything else I haven't thought about?

[AbrilRBS]

This was a typo in the test conanfile, but the behaviour is identical in these tests

[AbrilRBS]

The thing is that as long as these functions are called inside the source method (For the patch case 100%, replace_in_file() also has valid use-cases inside the package method, but it's a diferent deal!), this would not be a problem

The issue arises when recipes call these methods in the build() method - CCI is almost exclusively like this, with few recipes doing it in the source() method, probably as a byproduct of old Conan 1 behaviour

[memsharded]

I am not sure if I understand.
Patches and replace_in_file should be applied in general in the source() method, over the source folder. So they shouldn't warn by default.

The fact that ConanCenter patches unnecessarily in the build() method is what should be updated, most of them should patch in the source() method, as the patches are not configuration-specific.

[AbrilRBS]

We're going with the linter approach to be released soon, as this would create many false positives for users that are following the guidelines properly, and the added complexity of inspecting the stack to check for the build method is not worth it in our opinion

[jcar87]

The fact that ConanCenter patches unnecessarily in the build() method is what should be updated, most of them should patch in the source() method, as the patches are not configuration-specific.

Yes - this has always been a problem.
Patches must be configuration-agnostic (or have the build system conditionals in the patch) for them to be in source() - but arguably, the patches in recipes via apply_conandata_patches() should already have this property by definition because they are not "conditionally applied".

I suppose patching in build() needs to happen when there are conditional patches (conditional on options or settings) - sometimes it's easier to do that than to create a platform-agnostic patch - however, while that may be valid - it must not happen if no_copy_sources = True

[jcar87]

CCI is almost exclusively like this, with few recipes doing it in the source() method, probably as a byproduct of old Conan 1 behaviour

Conan 1 documentation always advocated for doing this in the source() method as well - and the source folder is deemed immutable once the the source() method has finished, but that the contents of the source folder should be platform (including settings and options) agnostic.

Conan 2 does have that temporary copy of the source folder during build - so platform dependent changes are "safe" in the build() method. I think it's a mix of that, alongside a mentality of "the source() method should be the upstream sources, untouched" - but I think the conandata.yml already has a clear source of truth for that anyway. And for traceability purposes, having the source() method have what's actually being built, can be convenient when needed for compliance.