search

mypaint / libmypaint

libmypaint, a.k.a. "brushlib", is a library for making brushstrokes which is used by MyPaint and other projects.
http://mypaint.org
Other
308 stars 87 forks source link

Fix Name and Requires in libmypaint-gegl.pc. #165

Closed badshah400 closed 4 years ago

badshah400 commented 4 years ago
jplloyd commented 4 years ago

Thank you, I missed a spot there.

This fix should be made on top the libmypaint-v1.5.x branch and set up to merge against that same branch.

This needs to be fixed in master as well, but there -@LIBMYPAINT_API_PLATFORM_VERSION@ should not be removed, as it is currently set up so that both major and minor numbers are part of the package name (not very fond of that myself). Additionally, in master the platform version should also be appended to the end of the name (after adding -gegl).

badshah400 commented 4 years ago

I'll have a PR for the 1.5.x branch soon.

Jehan commented 4 years ago

@jplloyd I have not followed much, sorry for this. But did you change libmypaint 1.5 to generate libmypaint-1.5.pc or something similar? I hope not! libmypaint.pc was not the best choice, but now that it was done, you should stick with it for all the v1.x series. Otherwise a project which was requiring a libmypaint say 1.3.0 or above (e.g. GIMP) would not find libmypaint 1.5.0 anymore (and would need to tweak the configure script with if conditions and whatnot).

So basically libmypaint 2 and over will have versionning in the pkg-config (and other) file but stick to old naming convention for the v1 series (we just know that no version == v1.x).

it is currently set up so that both major and minor numbers are part of the package name (not very fond of that myself)

Are you saying that libmypaint 2.0 pkg-config file will be libmypaint-2.0.pc but v2.1 will be libmypaint-2.1.pc. Once again, I hope not! Unless you are going for a versionning scheme where minor versions are incompatible (i.e. away from semantic versioning). You should once again stick to a same naming for a whole serie meant to be backward compatible. It could be libmypaint-2.pc, libmypaint2.pc or libmypaint-2.0.pc or whatever, but you should stick to your name. Pick the convention you like (you still have time, stable libmypaint 2 is not out yet) and stick to it for years to come.

Note: the '0' in libmypaint-2.0.pc does not mean minor version of '0', but we are talking of the whole 2.x series. This is for instance the convention used by GTK+:

$ pkg-config --modversion gtk+-3.0 
3.24.13

If you don't like this '0' here (not sure either the purpose), just go for libmypaint-2 for instance.

badshah400 commented 4 years ago

@jplloyd I have not followed much, sorry for this. But did you change libmypaint 1.5 to generate libmypaint-1.5.pc or something similar? I hope not! libmypaint.pc was not the best choice, but now that it was done, you should stick with it for all the v1.x series. Otherwise a project which was requiring a libmypaint say 1.3.0 or above (e.g. GIMP) would not find libmypaint 1.5.0 anymore (and would need to tweak the configure script with if conditions and whatnot).

So basically libmypaint 2 and over will have versionning in the pkg-config (and other) file but stick to old naming convention for the v1 series (we just know that no version == v1.x).

I think this is what @jplloyd meant too. That's why I made a separate pr #166 for the 1.5.x branch.

jplloyd commented 4 years ago

@jplloyd I have not followed much, sorry for this. But did you change libmypaint 1.5 to generate libmypaint-1.5.pc or something similar?

No, it's still libmypaint.pc in the libmypaint-v1.5.x branch, and the corresponding -gegl pc file is libmypaint-gegl.pc.

The concerns for the current (and 1.5.0) libmypaint-gegl.pc are:

Are you saying that libmypaint 2.0 pkg-config file will be libmypaint-2.0.pc but v2.1 will be libmypaint-2.1.pc.

That is how it is set up now in master, since this commit: 0b31421ffbfb5f4a1c68ceeafa292c6ff08e949c nearly three years ago. To quote myself from earlier in the discussion:

(not very fond of that myself)

The commit references this issue: #92, which mentions a discussion on the GIMP irc channel. I don't know if you were involved in that discussion of course, but maybe there was some confusion on how to do things.

I was already pretty set on reverting the "at the level of the minor version number" part, so I'm glad you agree.

Jehan commented 4 years ago

I was already pretty set on reverting the "at the level of the minor version number" part, so I'm glad you agree.

Yeah I do agree and any other GIMP dev will. So if that were the result of some earlier discussion with us, it's just a misunderstanding. ;-)

If you look at our own configure file for instance (for libgimp* which are libs used by plug-ins), we have a separate variable gimp_pkgconfig_version, and it is set at "2.0" even though we are at the 2.10.16 version of GIMP (and libgimp). We don't use the application major/minor versions which are a different thing.

So yeah we should just change this. I can make the fix later on tonight, if you wish. Should we go with libmypaint-MAJOR.0.pc ?

jplloyd commented 4 years ago

So yeah we should just change this. I can make the fix later on tonight, if you wish. Should we go with libmypaint-MAJOR.0.pc ?

Unless there is a good reason for the .0 (other than being a gimp/gtk convention), I personally think libmypaint-MAJOR.pc is sufficient. You are very welcome to make the reversal if you want.

Question: seeing as the bugs in the libmypaint-gegl.pc file in 1.5.0 are build-breaking, I'm assuming I should make a 1.5.1 release with the fixes? Of course, no known project currently uses that component, but still.

@badshah400 I have squashed and merged your commits, editing the message to reflect the sum of the changes. This is primarily so the .MINOR-level reversal will be consistent. Thanks!

badshah400 commented 4 years ago

@badshah400 I have squashed and merged your commits, editing the message to reflect the sum of the changes. This is primarily so the .MINOR-level reversal will be consistent. Thanks!

Thank you very much.