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 -Werror controllable #49

Closed
wants to merge 1 commit into from

Conversation

vapier
Copy link

@vapier vapier commented May 17, 2016

Move the -Werror flag into a variable so we can disable it if need be.

Move the -Werror flag into a variable so we can disable it if need be.
@r0m30
Copy link
Contributor

r0m30 commented May 19, 2016

Hi,
This makefiles are created and controlled by the IDE (netbeans) that I use to develop in Linux. Changing them isn't really an option as the IDE believes that it owns them and will recreate them whenever it wants to.

Why would you not want -Werror ? That is a requirement for some distro packaging so we can't accept any code without it.

@vapier
Copy link
Author

vapier commented May 19, 2016

it isn't feasible or realistic to handle every single generated warning. there's no way you're going to validate every possible combination of compiler/c library/optimization/architecture out there. warnings in gcc (both good & false positives) change with every version and can differ strongly based on optimization flags or the target architecture behavior. many warnings too are triggered by the C library headers.

i don't have a problem with the git repo defaulting to -Werror. but it doesn't make sense to force all builds to always use it. my change also has zero impact on the default behavior, or on distros that want to enforce -Werror themselves.

@r0m30
Copy link
Contributor

r0m30 commented May 24, 2016

Did you see the first paragraph? The files you are modifying are generated. Patching them is a futile exercise, they will be replaced by the IDE whenever you make a change to the project.

@vapier
Copy link
Author

vapier commented May 24, 2016

then where do you want things fixed ? you're clearly the one in control, so you need to tell me how to fix this.

@r0m30
Copy link
Contributor

r0m30 commented May 26, 2016

How about this...
You use your patched files until the next release and when I create a new relesase I'll add two new configurations, one for 32bit with no -Werror and one for 64bit with no -Werror to sedutil and LinuxPBA.

@vapier
Copy link
Author

vapier commented May 26, 2016

hacking things locally (for Gentoo & Chromium OS) isn't an issue ... that's done. we can wait for the next release to drop the hacks and move to a better target.

if you're going to be tweaking things, it'd also be nice to drop the hardcoding of -O2 and -g in many places. distros doing their own builds will frequently want to use their own optimization settings.

@cristim
Copy link

cristim commented May 21, 2017

I forked the project into a new Github organization, and it's available at https://github.com/sedutil/sedutil

Please join us there and re-submit this PR, we promise to merge it timely after passing the code review.

@r0m30
Copy link
Contributor

r0m30 commented Jul 21, 2017

There is now a standard autotools build in the root directory, it builds both sedutil-cli and linuxpba by default but each has it's own target as well. You should be able to use the standard methods to override the defaults using this. It will be maintained because it's needed for the pba build. I'm no autotools wizard so if it needs to be changed to give you the flexibility you need patch away.

@r0m30 r0m30 closed this Jul 21, 2017
@vapier
Copy link
Author

vapier commented Jan 4, 2019

the build still hardcodes the -Werror flag which we don't want. should I file a bug for you to address it?

@vapier
Copy link
Author

vapier commented Dec 8, 2023

refiled as #453 then

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.

None yet

3 participants