Skip to content
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

Make the reinplace warning an error #232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kurthindenburg
Copy link
Contributor

This changes the warning "reinplace didn't change anything in" to an error. This will force maintainers to fix these issues. This can be bypassed by using the quiet (-q) option. Two tests are added to verify the code.

Closes: https://trac.macports.org/ticket/60844

This changes the warning "reinplace didn't change anything in" to an error.  This will force maintainers to fix these issues.  This can be bypassed by using the quiet (-q) option.  Two tests are added to verify the code.

Closes: https://trac.macports.org/ticket/60844
@ryandesign
Copy link
Contributor

It's an easy change to the code, certainly, and I do want to make this change, but it's going to break so many ports.

@mascguy
Copy link
Member

mascguy commented Oct 28, 2021

I'd prefer this to be an opt-in option, enabled via a new parameter.

This is similar in spirit to optional "strict" mode, used in some CMake (and/or Configure) scripts.

@catap
Copy link

catap commented Jun 15, 2023

Just an example of port where reinplace is used to adjust different files. Not all of them contains pattern: https://github.com/macports/macports-ports/blob/master/emulators/xhyve/Portfile#L31-L36

Another example of port which uses reinplace to clean up platform-specified traces to make universal variant: https://github.com/macports/macports-ports/blob/master/devel/icu/Portfile#L98-L116

@kurthindenburg do you see any another way to solve that issues after your patch? Call sed by hand instead of reinplace?

@barracuda156
Copy link

I think this should not be a default option. Totally agree with @mascguy – opt-in is fine.

@ryandesign
Copy link
Contributor

Just an example of port where reinplace is used to adjust different files. Not all of them contains pattern: https://github.com/macports/macports-ports/blob/master/emulators/xhyve/Portfile#L31-L36

Use reinplace's -q flag in this case.

Another example of port which uses reinplace to clean up platform-specified traces to make universal variant: https://github.com/macports/macports-ports/blob/master/devel/icu/Portfile#L98-L116

You can use -q here too if there's no better solution. A better solution might be to do the reinplace only if you know it will replace something. I haven't tried to determine if that's workable in that specific situation.

@catap
Copy link

catap commented Jun 16, 2023

@ryandesign thanks for -q option. It is that I'm looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants