openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.97k stars 2.55k forks source link

Rename addon.make, and restructure the content #3419

Open HalfdanJ opened 9 years ago

HalfdanJ commented 9 years ago

After been talking with @arturoc and @bakercp, i propose to change the make file for projects with 0.9.0.

There are several reasons to do this. First of all, the current file structure is very inflexible, and it would be good to add some more metadata to the file. First of all add more metadata to the addon dependencies, specifying specific git urls and version (optionally) to addon dependencies. This can be used by the addon manager tool cli tool (ofx) i'm currently working on for automatically handle the addons used in projects, an make sure you use the correct version of an addon.

Secondly renaming the file to config.mk. The current file name is confusing with the .make ending. It is used by the make system, but it's a very unconventional use of a .make file. The .mk file ending is more generic, and is also already used in the addons in the addon_config.mk file so it would make them look more like each other. Also using the same file structure as in the addon_config file for consistency.

The structure I propose is the following (based on feedback from arturo):

#These are optional, but a possibility, and taken from the addon_config.mk
meta:
    PROJECT_NAME = name
    PROJECT_DESCRIPTION = A project description
    PROJECT_AUTHOR = Jonas Jongejan
    PROJECT_URL = http://github.com/HalfdanJ/myProject

common:
    # dependencies with addons, a list of them separated by spaces 
    # or use += in several lines

        # Specify just addon name 
    ADDON_DEPENDENCIES += ofxKinect

    # Specify url and tag/sha
    ADDON_DEPENDENCIES = ofxSniffer:0.1@https://github.com/halfdanj/ofxSniffer

    # Specify just tag/sha
    ADDON_DEPENDENCIES += ofxCv:0.1

    # Variants: 

    # Matches 0.1.1 0.1.2 0.1.9
    ADDON_DEPENDENCIES += ofxCv:0.1.x   

    # Matches all versions after 0.1
    ADDON_DEPENDENCIES += ofxCv:>0.1

    # Matches all versions after 0.1 included
    ADDON_DEPENDENCIES += ofxCv:>=0.1

    # Matches all versions but 0.1
    ADDON_DEPENDENCIES += ofxCv:!=0.1

The syntax of specifying url / tag / sha should also be implemented in the addon_config.mk file

This should be implemented in the make system with backward compatibility, so it falls back on looking for a addon.make file.

What places should these changes be implemented? I can think of the following:

What do people think?

Edit: added greater then, less then syntax

ofZach commented 9 years ago

sounds good to me, but isn't the current system using .mk also and have files that are similar?, see for example these files which are similar to what you suggest already (renaming them seems like a good idea)

https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxOpenCv/addon_config.mk https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxKinect/addon_config.mk

it looks like there's also a PR to the ofxAddons template that has that file also -- so hopefully it should be part of the template soon

https://github.com/openframeworks/ofxAddonTemplate/pull/14

with regards to:

This should be implemented in the make system with backward compatibility, so it falls back on looking for a addon.make file.

I think maybe you are confusing this kind of file:

https://github.com/openframeworks/openFrameworks/blob/master/examples/addons/opencvExample/addons.make

which is a list of addons the project uses, vs the config file per addon?

ofZach commented 9 years ago

sorry, I totally misunderstood you -- see what you are suggesting now. Seems great to me! I've always felt that the addons.make file was a bit thin and it doesn't have alot of info in it. We can certainly autogenerate this file via PG.

bakercp commented 9 years ago

Also +1 on this. The trick is making sure that we can parse the git URL syntax in make without loads of escape characters. Ideally IMHO this would all be specified in json, but cross platform json parsing in the make system is extremely tricky without installing loads of command line tools. The current unconventional system was the best way to to do it given our primary design requirement of ease of use with a 35+ year old make build sytem :)

bilderbuchi commented 9 years ago

Nice, :+1: to this. I'd like to propose one minor addition: the version specifier syntax using the colon should be expanded with < > <= >= when using version numbers, this would add some very useful flexibility. At the same time, the : could probably also be replaced by a = to make this more consistent. See other version specification schemes for comparison.

Also, a wild thought: could we make the proposed file a dependency of the build/make process, and add an additional build step in which some script generates a make-valid file from this file, which then is used during the rest of the build process? This would free us from the constraints of keeping everything make-parseable. @arturoc?

HalfdanJ commented 9 years ago

@bakercp Yeah, it's an annoying limitation the 35+ year old make system. But i guess we are rather stuck with it. I guess that the url can be filtered out in the makefile with an rather simple regex? Something like ^([^:]+)

@bilderbuchi i'm not sure i understand the : replaced with =. I agree on the < >, let's add it in now, and start a possibility of proper versioning of addons in the future. I guess there might be a problem with the =, since it will conflict with the makefile syntax?

bilderbuchi commented 9 years ago

@HalfdanJ sorry for the brevity, I was on the phone. What I proposed is to add a set of version comparison operators, i.e. <, >, <=, >=,... to make the version parsing more useful. When we do this, there's a slight inconsistency between the set of comparison operators above, and the "equals" operator, which is currently : (e.g. openCv:1.1). This could be fixed by replacing :by = (but, as you say, a better choice would probably be ==). Also, to complete the set, we could/should also add the not-equals operator, !=, barring parsing conflicts.

For one external reference, the set of Python's package manager pip's requirement specifiers can be found here:

comparison ::= '<' | '<=' | '!=' | '==' | '>=' | '>'

pizthewiz commented 9 years ago

Similarly, the npm dependencies specification for package.json might be relevant here too:

I am all for these updates, but along the lines of what @bakercp said, if we are revamping the file content and structure, is it a good time to reconsider the format? If we largely control the tool consumers (projectGenerator, make, OFPlugin, ofxaddons.com …) of these files, what is anchoring us to make and not something more expressive like JSON or XML (not my preference but still worth discussing)?

For what it is worth, Cinder uses XML files to describe CinderBlocks (CinderBlock:Cinder :: Addon:OF) and is read by TinderBox (TinderBox:Cinder :: projectGenerator:OF) for project generation purposes - the format might be worth a quick look:

HalfdanJ commented 9 years ago

I agree that it's very much worth considering if it's time to move completely away from the make/mk file. JSON would definetly be a much more modern solution.

I have a question to @arturoc, what OS'es uses the makefile config.project.mk? All OS'es, or only some? I have tried to follow the files, but i don't get it completely.

bakercp commented 9 years ago

IMHO it's not time to move away from the make system, mostly because it works :). We just recently (in the last year or two) overhauled it, documented and added a pretty sophisticated (albeit difficult to grok) and extensible system. It's the main reason we're able to quickly adopt and support new ARM platforms with minimal re-configuration. Currently the make file system works for win_cb/codeblocks, all versions of linux, OSX, android, ios (with a few updates), etc. During the redesign we set it up to allow new platform variants to be easily supported (all of this is extensively documented in the makefiles themselves). Certainly it could be even clearer (there is a PR out there that has even more documentation in it ... but I haven't had time to clean it up and resubmit it).

Anyway, a lot of research has gone into supporting more modern make systems (cmake, premake, etc etc), but have been met with resistance at various points. The main advantage of using our existing system is that is just works without installing additional command line tools, etc. For example -- the make system is what powers ofSketch on osx, linux and windows. The current makefile system isn't perfect of course, it's super slow in some cases, and there are a lot of redundancies and potentially unnecessary recursions ... but again ... it works :)

So, I'm totally in favor of adding the addon dependencies syntax to the addons_config.mk and addons.mk -- we need that! But I'd passionately discourage moving away from make until we absolutely have to -- which I don't see happening for a while longer. But that is probably because I have a lot of time invested in it over the last few years and know how complex it can get ... so take that for what you will.

That said, I'd heartily encourage us to keep this issue focussed on integrating the proposed changes (git versioning, dependencies etc) into the makefile system rather than creating a new build system ...

P.S. If people are interested in say -- cmake -- I know @procedural has worked on a cmake build lately ...

HalfdanJ commented 9 years ago

@bakercp point taken :)

bakercp commented 9 years ago

@HalfdanJ I really do appreciate your work on this! ... Sorry If I came across as a super conservative curmudgeon :)

pizthewiz commented 9 years ago

No that is my fault, I hijacked this a bit - I'd been thinking about higher-level addon descriptions and was thinking if we are asking addon authors to make changes to support new machinery (more likely, we are asking them to write config.make files to begin with), it might be good to think about the runway the new machinery afforded us.

Regardless, I think the proposed changes are good!

bilderbuchi commented 9 years ago

I think two things got conflated here - make as a build system, and the addon description file, which is currently restricted by having to adhere to the make syntax. I also think we should not get rid of make as a build system (although e.g. cmake could also be worth exploring). @bakercp do you see any reasonable chance that we could use some (shell)script to generate, in an additional build step, a make-compatible description file like our current addons.make from an "extended" file like proposed here, ready for consumption by make? Then we could have the best of both worlds - have one metadata file to describe both the build params and the dependency,... info.

bilderbuchi commented 9 years ago

To just throw an idea out there, one approach would be to for example have the additional info which would confuse make in a make-comment section (i.e. lines starting with #). Then make would not see that info, and any more sophisticated/complex consumer of that file could strip out the make comment chars and parse the rest as-is. Granted, it would make the syntax of the file a bit strange/clunky, but it could be a low-tech solution to this problem.

pizthewiz commented 9 years ago

Interesting suggestion, @dlublin's Interactive Shader Format (ISF) does this and includes a JSON blob in a comment as the descriptor, see the section The ISF "specification" for more detail, here is a quick example:

/*{
    "DESCRIPTION": "demonstrates the use of float-type inputs",
    "CREDIT": "by zoidberg",
    "CATEGORIES": [
        "TEST-GLSL FX"
    ],
    "INPUTS": [
        {
            "NAME": "inputImage",
            "TYPE": "image"
        },
        {
            "NAME": "level",
            "TYPE": "float",
            "DEFAULT": 0.5,
            "MIN": 0.0,
            "MAX": 1.0
        }
    ]
}*/

void main()
{
    vec4        srcPixel = IMG_THIS_PIXEL(inputImage);
    float        luma = (srcPixel.r+srcPixel.g+srcPixel.b)/3.0;
    vec4        dstPixel = (luma>level) ? srcPixel : vec4(0,0,0,1);
    gl_FragColor = dstPixel;
}
HalfdanJ commented 9 years ago

Is this something we should add to the 0.9.0 milestone? I can do most of the changes myself, but the makefile i need some help with, since i'm not sure how to test it properly.

Regarding @pizthewiz idea, the only problem i see with that is you then need to add the addon name two places in if you handwrite the file, since the makesystem still can't read the JSON blob. Or am I wrong?

kylemcdonald commented 9 years ago

just catching up on some of this now, but wanted to say: any contributions that aren't slated for the upcoming release are always welcome :) but we can't add anything new to that list -- trying to keep people focused :)

not sure about reading json blobs with make. i think bakercp and arturo know the makefile system well, but bakercp is focused on apothecary right now and i think arturo is busy with a project at the moment. might have to wait a little on this one, unless someone else has some time to chat with you.

On Tue, Nov 25, 2014 at 11:12 PM, Jonas Jongejan notifications@github.com wrote:

Is this something we should add to the 0.9.0 milestone? I can do most of the changes myself, but the makefile i need some help with, since i'm not sure how to test it properly.

Regarding @pizthewiz https://github.com/pizthewiz idea, the only problem i see with that is you then need to add the addon name two places in if you handwrite the file, since the makesystem still can't read the JSON blob. Or am I wrong?

— Reply to this email directly or view it on GitHub https://github.com/openframeworks/openFrameworks/issues/3419#issuecomment-64512701 .

mruegenberg commented 9 years ago

It would be really nice if the new structure supports platform-specific addons (ie make the TARGET_xyz defines available in the file).

A concrete usecase is the ofxAndroid addons. I have a project that runs on both Android and the PC, but I always get a warning about ofxAndroid and ofxAccelerometer missing when compiling on other platforms.

arturoc commented 9 years ago

android already adds those addons by default so they don't need to be in the addons.make config file that way it'll compile in both. but yes in being able to specify config.make and addons.make per platform would be useful

mruegenberg commented 9 years ago

Oh, ok. In that case the addons.make in androidEmptyExample should probably be updated to be empty, since it contains both of those.

Are you sure that ofxAccelerometer is also added? config.android.default.mk only defines PLATFORM_REQUIRED_ADDONS = ofxAndroid.

mruegenberg commented 8 years ago

Is there any progress on platform-dependent addon lists? Can I use config.mk? Just asking because I just ran into this issue again with ofxSpout (which makes sense only on Windows) and ofxSyphon (only on OS X) on a cross-platform project.