Skip to content

Conversation

HalfdanJ
Copy link
Member

Recursively add addon dependencies Fixes #30

@ofZach
Copy link
Contributor

ofZach commented Nov 11, 2015

thanks this is great ! I'd like to check how the PG handles multiple addons, like if ofxCv include ofxOpenCv but ofxOpenCv is already in the addons.make file, does it get included twice? I am wondering if need some smarts or not....

@HalfdanJ
Copy link
Member Author

It needs a check yes. And i just realized that the make system has the same problem (just tried with ofxCV depending on ofxOpenCv..)

@arturoc
Copy link
Member

arturoc commented Nov 12, 2015

instead of adding a check it's easier and cleaner to use a set for the collection of addons that way every addon no matter how many times is added will be unique. the return for the method that gets the collection of addons can still be a vector that gets filled from the set

@HalfdanJ
Copy link
Member Author

I just looked into this, the check is already there: https://github.com/openframeworks/projectGenerator/blob/master/ofxProjectGenerator/src/projects/baseProject.cpp#L261

Should i change it to the set solution @arturoc mentions, or should we keep it as it is?

@bakercp
Copy link
Member

bakercp commented Jan 31, 2016

Just wanted to ping this issue -- any way I can help move it though?

@bakercp
Copy link
Member

bakercp commented Nov 2, 2016

Ping. Would love to see this merged ...

@arturoc
Copy link
Member

arturoc commented Nov 2, 2016

i think this might need some work still to avoid duplicated addons. i think adding the addons in a set as a first step to then add them should solve any potential problem. i guess we could have a public method that is

addAddons(vector<std::string>)

so all addons have to be added at once and then that method takes care of parsing the dependencies and finally add the individual addons one by one calling the now private addAddon

the command line tool already has a vector for the addons so the change would be trivial

@bakercp
Copy link
Member

bakercp commented Jan 15, 2017

This is becoming more important now for automated unit testing etc.

@thomasgeissl has done some work on this interesting project that seems related -- https://github.com/thomasgeissl/ofPackageManager

While I'm not super interested in creating a npm / bundler-style dependency manager it would be really nice to be able to handle more slightly more complex dependency lists in the addon_config.make file and the addons.make ...

At the very least it seems that we should be able specify the github user / repo e.g. bakercp/ofxHTTP and branch / tag. Thoughts on the best way to do this? (related #30)

@bakercp
Copy link
Member

bakercp commented Jan 15, 2017

Here's a nice well-articulated syntax that I'd love to support in addon_config.mk and addons.make files:

https://docs.npmjs.com/files/package.json#git-urls-as-dependencies
https://docs.npmjs.com/files/package.json#github-urls

And we kind of already support this with the local_addons syntax, etc.
https://docs.npmjs.com/files/package.json#local-paths

@ofTheo
Copy link
Member

ofTheo commented Nov 8, 2019

Just ran into this - would love to get this / #30 fixed.
I can see the possible recursion issues people mention.

Probably just checking the addons vector before adding and ignore duplicates will solve this.
I can submit a PR to your branch @HalfdanJ .

@ofTheo
Copy link
Member

ofTheo commented Nov 8, 2019

Actually this looks fine as is. It avoid duplicates at the start of that function already.
Going to merge this - there might be better ways in the future but this is a good start.

@ofTheo ofTheo merged commit 0e6823f into openframeworks:master Nov 8, 2019
@HalfdanJ HalfdanJ deleted the addon_dependencies branch November 22, 2019 16:58
@autr
Copy link

autr commented Feb 29, 2020

Is this in the latest 0.11 release? I doesn't happen with the bundled PG...

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

Successfully merging this pull request may close these issues.

ADDON_DEPENDENCIES from addon_config.mk is not processed

6 participants