search

swami / libinstpatch

Instrument file software library.
Other
20 stars 6 forks source link

Target naming #8

Closed jjceresa closed 5 years ago

jjceresa commented 5 years ago

This PR address discussion in issue https://github.com/swami/libinstpatch/issues/3 about:

derselbst commented 5 years ago

Having a libinstpatch-1.so on linux seems wrong. On linux the version is usually appended after the extension like libinstpatch.so.1.1.0. Suffixing the major version to the filename was meant for windows only. Sry, for the misunderstanding. I can fix this for linux, but this may take some time.

Also, I'm not quite sure whether it's correct to bump LIB_VERSION_CURRENT. This would indicate a API/ABI breakage on linux, which currently doesn't exist. I need to think about that.

jjceresa commented 5 years ago

Sry, for the misunderstanding. I can fix this for linux, but this may take some time. Also, I'm not quite sure whether it's correct to bump LIB_VERSION_CURRENT. This would indicate a API/ABI breakage on linux, which currently doesn't exist. I need to think about that.

No problem. As it is a minor issue easy to fix, this can wait until we adopt a convention.

jjceresa commented 5 years ago

Also, I'm not quite sure whether it's correct to bump LIB_VERSION_CURRENT. This would indicate a API/ABI breakage on linux, which currently doesn't exist.

You're right. In fact, i was confused by fluidsynth case in which major library version (LIB_VERSION_CURRENT) is equal to major package version (i.e IPATCH_VERSION_MAJOR). I feel wrong and still don't see the purpose ofpackage versionand the relation (if any) between 'library version and package version.

derselbst commented 5 years ago

I've fixed library naming on unix. There now is a libinstpatch.so. Please test whether there is a libinstpatch-0.dll on windows. Also I've removed the custom defined variables from libinstpatch.pc.in. Although supported, I don't think it's a good idea to use custom defined variables here.

jjceresa commented 5 years ago

I've fixed library naming on unix. There now is a libinstpatch.so.

Ok.

Please test whether there is a libinstpatch-0.dll on windows.

All is fine here. Tks.

Also I've removed the custom.....Although supported, I don't think it's a good idea to use custom defined variables here.

Ok. I am fine with this target naming PR. Please, consider this ready to be merged.