search

gearman / gearmand

http://gearman.org/
Other
737 stars 138 forks source link

Gcc 8+ and 11 fixes #350

Closed SpamapS closed 1 year ago

SpamapS commented 1 year ago

While testing gearmand with newer versions of Ubuntu (20.04 and 22.04) I found a few minor things that needed updating in order to build and test.

esabol commented 1 year ago

Also, please don't release a new gearmand without looking at PR #348. 😄

esabol commented 1 year ago

This PR is failing to pass the CI. Any idea why?

I think this pragma only works on gcc >= 8, but surely ubuntu-latest has gcc >= 8?

I think we'll need #if __GNUC__ >= 8 put around these pragmas to continue to support older gccs. I really should get PR #334 in a state where it can be merged in order to test this on the full range of supported versions of gcc.

SpamapS commented 1 year ago

Actually I started this on 20.04 which is why it was called gcc8 but then I upgraded to 22.04 so now it's actually gcc11 for that specific one. ubuntu "latest" I think is still 20.04 and so gcc8 which is why the one failed.

esabol commented 1 year ago

Actually I started this on 20.04 which is why it was called gcc8 but then I upgraded to 22.04 so now it's actually gcc11 for that specific one. ubuntu "latest" I think is still 20.04 and so gcc8 which is why the one failed.

Ah, OK. That makes sense!

... except I've tested gcc-8 builds previously and it worked fine in my testing, so I'm not sure I understand why you saw some failure now.... Ah, maybe because you were testing gcc-8 on Ubuntu 20.04, and I've been testing gcc-8 on Ubuntu 18.04?

SpamapS commented 1 year ago

Thanks Ed. Added those guards. I agree it would be good to have GH actions for all the gcc's we support. This whole warnings as errors business is really tedious to support across so many distros. Might be good to think about dropping some old ones.

esabol commented 1 year ago

Might be good to think about dropping some old ones.

Please don't say that after I spent so many, many hours getting the CI workflow to build with gcc 4.8 and 4.9. 😆

This is looking good. Two options on how to proceed:

(1) I remove the gcc-11 build from PR #334 and we merge that. Then you rebase PR #350 on the new master and add the gcc-11 build to the CI workflow in this PR.

(2) You merge PR #350 and then I rebase PR #334 on top of it. Less work, but it assumes PR #350 won't break any other builds.

Which option do you prefer?

esabol commented 1 year ago

Oh, it seems I'm too slow to comment. Never mind. Guess we are going with option 2. :smile:

SpamapS commented 1 year ago

I'm moving kind of fast as I'd like to push a release out today if everything looks good. Using the new dockerfiles to test locally. :)

On Sun, Nov 13, 2022 at 10:06 AM Ed Sabol @.***> wrote:

Oh, it seems I'm too slow to comment. Never mind. Guess we are going with option 2. 😄

— Reply to this email directly, view it on GitHub https://github.com/gearman/gearmand/pull/350#issuecomment-1312788344, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADS6YDNBZ6PR5KO7E2UYFDWIEU37ANCNFSM6AAAAAAR6SEIBY . You are receiving this because you modified the open/close state.Message ID: @.***>