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

Fix for #92 causes ABI break #101

Closed
sur5r opened this issue Aug 14, 2017 · 11 comments
Closed

Fix for #92 causes ABI break #101

sur5r opened this issue Aug 14, 2017 · 11 comments
Milestone

Comments

@sur5r
Copy link

sur5r commented Aug 14, 2017

0285479 introduced a new field comment in the middle of struct cfg_t which causes an ABI break.

See

Seems #81 does not catch this.

@DaveDavenport
Copy link

DaveDavenport commented Aug 14, 2017

It is mentioned here in the 3.1 release. (but as medium)

Is something like libtool versioning not smart to use?

@wRAR
Copy link

wRAR commented Aug 14, 2017

Unfortunately, the libtool doc doesn't teach which changes are backward compatible and which aren't. Changing field offsets in a struct isn't.

@DaveDavenport
Copy link

No it does not. (But it still has added value as it provide more info for the rt linker.)

@troglobit
Copy link
Collaborator

So, move the comment field to the end and keep the ABI version or bump it for a v3.2.1 bug fix release?

@sur5r
Copy link
Author

sur5r commented Aug 15, 2017

As both 3.1 and 3,2 are "out there" I would consider a bump the correct fix. That aside, I would not move the comment field around unless there are other reasons to do so.

@DaveDavenport
Copy link

I would strongly recommend against an abi break to fix an abi break.
IMHO a bump would be the right fix.

(p.s. thanks for the great library).

@troglobit
Copy link
Collaborator

OK, ABI bump for v3.2.1 seemed the sane choice for me as well. But with so many interested parties I thought it only prudent to ask.

I'll get right on the release work in a day or two.

It's going to be the last release in a while, so if you guys have anything else you want fixed, it's time to pipe up :)

Otherwise, thanks for the awesome report and the kind words!

@sur5r
Copy link
Author

sur5r commented Aug 15, 2017

OT for this issue, but, as you asked and before I open a new one:

There is Debian #462251 sitting around and I can't find it among the closed github issues.

@troglobit
Copy link
Collaborator

@sur5r Ah, that one ... yeah someone should double check that myvar = "${HOME}/path" is still an issue before we at least file a corresponding issue here.

@troglobit troglobit added this to the v3.2.1 milestone Aug 15, 2017
@troglobit
Copy link
Collaborator

I've checked this now, Debian #462251 was fixed by @fhunleth in commit 80fad81, which was released in libConfuse v2.8. We even have a unit test for it.

Debian has some catching up to do. I'll send an update to the Debian bug shortly.

troglobit added a commit that referenced this issue Aug 17, 2017
Fix #101: Bump ABI 1.1.0 --> 2.0.0 and prepare release v3.2.1
@troglobit
Copy link
Collaborator

There, fixed and relased v3.2.1.

Big thank you to everyone that contributed to to this!

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

No branches or pull requests

4 participants