enhance apply_regex_substitutions to allow specifying action to take in case there are no matches

[Flamefire]

• Fix a bug where the passed list was modified leading to outputs of '<_sre.SRE_Pattern..>' when the same list is passed in again
• Minor speedup by only using replacements when a match was found
• Make the log more readable by aggregating the replacements into a single output
• Add on_missing_match parameter with warn as default (from --strict) to catch outdated or faulty replacements but give invokers control over what should be done

[Flamefire]

Just wanted to highlight that this is expected to print warnings on current ECs/EasyBlocks. E.g. just got one from Bazel 3.4.1:

WARNING: Nothing found to replace in tools/cpp/cc_configure.bzl

It's not a hard failure but incentivizes checking why those replacements don't apply: Either an upstream fix (so no longer required), an upstream change (so need to be adjusted) or a mistake in the regexps.

Question also: Currently I warn only when none of the replacements applied. Maybe this should be enhanced to be more fine granular: Like WARNING: 1 out of 4 replacements not found in tools/cpp/cc_configure.bzl

[akesandgren]

That would probably be a good idea. But perhaps leave for a later enhancement unless it is trivial?

[Flamefire]

That would probably be a good idea. But perhaps leave for a later enhancement unless it is trivial?

It's not to hard but likely leads to more warnings. As mentioned I think that's ok.
Shall I go ahead?

[boegel]

That would probably be a good idea. But perhaps leave for a later enhancement unless it is trivial?

It's not to hard but likely leads to more warnings. As mentioned I think that's ok.
Shall I go ahead?

We don't want to start producing a large amount of warnings for what's basically no-op actions. If the patching no longer works but doesn't cause trouble, it shouldn't result in a verbose warning in the eb output I think.

That already applies here, regardless of whether we make the warnings more fine-grained or not...

[Flamefire]

If the patching no longer works but doesn't cause trouble, it shouldn't result in a verbose warning in the eb output I think.

But what if the patching is important and the trouble is only detected much later when the software is actually used? E.g. for most *Make builds we don't even run tests...

[boegel]

@Flamefire Can you revive this by fixing the merge conflict?

[Flamefire]

Rebased and resolved, hope that works

[boegel]

If the patching no longer works but doesn't cause trouble, it shouldn't result in a verbose warning in the eb output I think.

But what if the patching is important and the trouble is only detected much later when the software is actually used? E.g. for most *Make builds we don't even run tests...

That's a fair point, but the only way to catch issues like that is to get better at testing, not producing scary looking warnings for things that most likely don't matter. Usually if the patching silently failed it'll either trigger an installation error or it doesn't matter at all, I expect. There may be other cases, but we need to detect those in some other way imho.

Just logging a warning (log.warning rather than print_warning) would be fine though...

[boegel]

@Flamefire This will need some conflict resolving after the merge of #3507

[Flamefire]

Rebased. Wasn't particularly great to resolve the conflicts. Please doublecheck the result

[akesandgren]

@Flamefire More conflicts to resolve.

@boegel What is your take on this now?

suggested changes no longer relevant