search

jamoma / JamomaCore

Jamoma Frameworks for Audio and Control Structure
Other
36 stars 14 forks source link

Feature/extension check #383

Closed jcelerier closed 8 years ago

jcelerier commented 8 years ago

Hello,

Here is a small pull request that intends to fix the problems that Trond is encountering wrt old Jamoma dylibs present in the default search paths, which is a requirement for people using jamoma 0.5.

It just adds a check against to the git commit hash when loading the dylibs and will skip dylibs that don't have the data embedded.

@lossius could you try it ?

tap commented 8 years ago

I have two comments / questions:

  1. Do we understand why the CI builds failed? Is this safe to merge with those failures?
  2. Do we care about the following?

If I understand correctly (do I?), this change means that all extensions must have the same git rev as the Foundation lib itself. This defeats the entire purpose of compiling extensions as dynamically loaded libraries. You can no longer write an extension that is housed outside of the canonical JamomaCore repository and also you must compile everything each and every time.

In the initial Jamoma2 prototypes we have taken a different approach (e.g. the library is header-only) and we don't compile dynamic extensions like this -- so perhaps this is not a problem. Or maybe it is. I simply want to make sure we are thinking this through.

For the comparison to Git Rev I wonder if there is a reasonably easy way to check against an atomically incrementing number so that we can do a > comparison instead of an == comparison?

jcelerier commented 8 years ago

Hi,

I think that the git CI builds failed because I based this PR on a commit that wasn't building itself; to check I'll rebase it on the latest state of the master branch.

Of course, any better heuristic to check compatibility between extensions would be welcome, but in the end I don't think that it would be possible to bypass the developer actually "writing down somewhere" that the API / ABI contract was broken by increasing a version number (wasn't there talks of using semantic versioning for jamoma?).

For instance, an extension could say : "I work with versions ranging from jamoma core 1.0.23 to jamoma core 1.2.17" (or just wouldn't specify an upper range)?

I guess many projects do this with version macros...

(Also, this aims to solve a problem that appears between current versions of Jamoma and older ones cohabitating - not happily - together. I don't think that this would apply at all in header-only libs, but apart from Trond I know of at leaste one other person who would like to have Jamoma 0.5 and 1.0 installed on the same computer. And a Jamoma 0.5 Minuit.dylib certainly does not behave correctly on current Jamoma !).

tap commented 8 years ago

Looks like there is a MIDI extension in Modular that can't link.

tap commented 8 years ago

@jcelerier : Is this pull request still desirable? Should I close it? Or wait to see if it can be fixed easily? Thanks!

jcelerier commented 8 years ago

I haven't had much time to give to it and nobody re-asked me so I guess this can be closed without problems.