Addons parsing improvement (dont merge yet) #578

roymacdonald · 2024-09-25T20:32:09Z

This is what I have done so far related to this

I have encapsulated the ofAddon class so it is easier to test that it is loading/parsing properly the values for any addon.

I have dealt partially with the macos/osx issue. I say partially because I have not tested it thoroughly and it mostly feels like a hack. But so far it works for me, I am able to use addons, local addons, addons that use libraries, xcframeworks. Still neet to test for those using frameworks, system frameworks and dylibs. Currently only testing on macos 14 (sonoma) and xcode 15.

I will setup my windows/linux computer to be able to test on those other environments as well.

The main idea behind all of it is that an addon is an addon regardless of where it sits in the filesystem. Since all the routes are relative it should not matter where these sit, thus the local addons are treated in the exact same way as the non local ones.

Then the baseProject class does not do much with these besides loading and then it is the job of each baseProject-derived-class to override the necessary functions so it adds the addons properly.

There is a lot to be cleaned and reduced since there is a lot of duplicated code and stuff not even used.
Eventually it should provide a much cleaner way of dealing with addons and project creating in general.

…e abstract, and overriding these in derived classes

…nce string arg

…e different elemetns an added addon. Classes extending baseProject should not call addAddon or override it, instead should override these added functions

…sToFolders

… to it.

… macos instead of osx

… purposes.

…the addon's path

commandLine/src/projects/CBLinuxProject.h

ofTheo · 2024-10-03T15:39:19Z

@roymacdonald has this been tested more since the PR?

For testing Frameworks:
https://github.com/astellato/ofxSyphon

For testing dylibs:
https://github.com/design-io/ofxOrbbec

I'd love to merge this but would be good to know how much it's been tested across platforms ( as there are so many changes it's hard to review ).

… override declared

… rules to libfiles

…d, not following the general pattern

…tion

roymacdonald · 2024-10-29T20:37:49Z

dimitre · 2024-10-29T22:28:17Z

Nice. I think we should have it merged before resolving dylibs.
the problem with dylibs is most of the time they use some built in paths. like in the case of @rpath/ffmpeg/lib/osx/libavutil.56.dylib in ofxHapPlayer

maybe it needs more discussion. my opinion is we should remove all paths from dylibs and use them in a specific copy phase, I don't remember if frameworks or executables. it is easier to correct paths this way and dylibs find each other correctly if we use this way.

roymacdonald · 2024-10-29T22:56:35Z

The "@rpath" issue can be handled by the addon_config.mk via LD_FLAGS. Actually, I've been testing with ofxHapPlayer and it compiles and runs without any extra setting. In that particular case, since it sets the @rpath to the dylibs that are in the addon's folder it works with make command too.

I think that dealing with the libs paths should be handled by each developer that wants to use a dlyb in their addon/app, and we should provide instructions on how to achieve such easily.

artificiel · 2024-10-30T00:17:26Z

this is a nice test: https://forum.openframeworks.cc/t/addon-makefile-clash/44076

ofTheo · 2024-10-30T01:55:29Z

Yeah definitely fine to merge @roymacdonald - especially as we are not regressing with the make file dylib stuff.
Really excited for this one!

roymacdonald · 2024-10-30T15:10:41Z

this is a nice test: https://forum.openframeworks.cc/t/addon-makefile-clash/44076

I was actually testing this 2 days ago. it only afects make for some reason and not xcode. It requires a small change in the makefile to make it work. I will add a PR

roymacdonald · 2024-11-02T18:55:56Z

I will merge this. It seems that there are a lot of current issues that are solved here. Even when I have not tested it all I am confident that the areas taht I did not are the ones that I did not change.

…eeded in some cases

roymacdonald · 2024-11-02T19:24:05Z

damn, I don't have write access here. @ofTheo @dimitre , can you merge please?

dimitre · 2024-11-02T20:32:09Z

commandLine/src/projects/CBLinuxProject.h

-	void addLibrary(const LibraryBinary & lib){};
+	virtual bool createProjectFile() override;
+	virtual void addInclude(const fs::path & includeName) override{};
+	virtual void addLibrary(const LibraryBinary & lib)override{};


we can remove virtual here once it is final

I just updatetd it. THanks for pointing out. Although, the linux test fails immediately when trying to pull ubuntu

Thank you @roymacdonald let's merge it.

dimitre · 2024-11-02T21:21:50Z

I've noticed one error here compiling with make:

duplicate symbol 'toString(LibraryBinary const&)' in:
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/android2024.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBWinProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBLinuxProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/main.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/baseProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/utils/Utils.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/VSCodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/addons/ofAddon.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/androidStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/visualStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/xcodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/qtcreatorproject.o
duplicate symbol 'toString(std::__1::__fs::filesystem::path const&)' in:
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/android2024.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBWinProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBLinuxProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/main.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/baseProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/utils/Utils.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/VSCodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/addons/ofAddon.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/androidStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/visualStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/xcodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/qtcreatorproject.o
duplicate symbol 'toString(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&)' in:
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/android2024.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBWinProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/CBLinuxProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/main.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/baseProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/utils/Utils.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/VSCodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/addons/ofAddon.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/androidStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/visualStudioProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/xcodeProject.o
    /Volumes/tool/ofw/apps/pgd/commandLine/obj/macos/Release/src/projects/qtcreatorproject.o
ld: 3 duplicate symbols

it seems to work ok if we change toString functions and removeDuplicates to static
I can to a PR with this and move this functions to inside ofAddon, because it is the only place that uses it

roymacdonald · 2024-11-02T21:37:57Z

Nice catch. I was not using make for building PG, nor tested it.
Making those static would be a good idea, but also putting those toString functions inside a namespace, something like of::priv:: which is used elsewhere.
If you can make those changes it would be great

dimitre · 2024-11-02T22:12:14Z

ok merged in another PR here #583
I'm manually generating a nightly build so we can inspect examples to see if they are OK in windows / mac

roymacdonald · 2024-11-02T23:02:19Z

awesome. Thanks @dimitre

dimitre · 2024-11-02T23:05:53Z

Thank you for the improvements in PG.
when you have time take a look at the issues to close the ones addressed in your PR

roymacdonald added 17 commits September 16, 2024 20:49

marking virtual override, set several functions in baseProject as pur…

a3681b2

…e abstract, and overriding these in derived classes

baseProject some functions now are pureabstract and have const refere…

b371258

…nce string arg

changing method to identify local addons

af588f4

adding virtual functions for the parsing and processing of each of th…

db2ac57

…e different elemetns an added addon. Classes extending baseProject should not call addAddon or override it, instead should override these added functions

correctly saving and loading addon_config

8b462cd

using correct relative paths

8abbe8a

corrected adddon Relative Path.

4a75fa6

moved redundant code into single function for adding elements to file…

351bc84

…sToFolders

fixing libraries paths even when it is a local_addon.

cae1fe0

cleanup + comments

4662abc

comments

47373ed

setting fs::current_path( to project's path so the paths are relative…

f0538d2

… to it.

encapsulating ofAddon loading.

92bd210

updating use of ofAddon.

06695f2

fixes to getFolderUUID so it properly handles local addons

7113d7c

Horrible hack to properly parse libs and frameworks on when these use…

e0860d1

… macos instead of osx

Merge branch 'openframeworks:master' into addonsParsingImprovement

b22c01c

roymacdonald mentioned this pull request Sep 25, 2024

macOS: nightly build PG can't add local addons started with relative path? #562

Open

roymacdonald added 8 commits September 25, 2024 19:50

dont remove comments from the addons in addons.make file

87c6fe6

added json output for ofAddon class. moslty for development debugging…

26160d3

… purposes.

removed some alerts

c254a2c

removing duplicates

a525826

changing LibraryBinary from struct to class

df54108

remove unnecesary alert

bb6b022

Removed noARC flag as it is being added improperly to some objc files.

1677f28

ofAddon: removed fromFS function and parsing name before normalizing …

6b32307

…the addon's path

dimitre reviewed Oct 2, 2024

View reviewed changes

commandLine/src/projects/CBLinuxProject.h Show resolved Hide resolved

ofTheo mentioned this pull request Oct 3, 2024

osx nightly signing errors for various libs openframeworks/openFrameworks#8135

Closed

removing redundant virtual declaration of functions that already have…

ebc0a3f

… override declared

roymacdonald added 10 commits October 21, 2024 22:35

removing duplicate libs, adding libs to projectFolders. apply exclude…

6b2b641

… rules to libfiles

xcodeProj. adding Dylib with folder

315f9c2

added missing include

ac763b5

ofAddon: made const strings for a bunch of strings that were hardcode…

525512e

…d, not following the general pattern

removed unused and redundant functions. replaced for a templated func…

2a8de70

…tion

libraryBinary auto convert to fs::path

0071b23

ofAddon: some cleanup and dealing with exclusions

dae2353

remove fixSlashOrder functions as std::filesystem::path deals with it

cf04c71

removing use of fixSlashOrder and using ofPathToString

b7b93dc

xcodeProject: some cleanup

34624c5

roymacdonald added 3 commits November 2, 2024 15:59

not normalizing libs paths, as that avoids copying links, which are n…

1c7d0cf

…eeded in some cases

merging with upstream

0957d79

bumped version number

5dfd6ae

dimitre reviewed Nov 2, 2024

View reviewed changes

removed unnecesary virtual keyword

c560c90

dimitre merged commit e9e9562 into openframeworks:master Nov 2, 2024
2 of 3 checks passed

roymacdonald deleted the addonsParsingImprovement branch November 2, 2024 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addons parsing improvement (dont merge yet) #578

Addons parsing improvement (dont merge yet) #578

roymacdonald commented Sep 25, 2024

ofTheo commented Oct 3, 2024

roymacdonald commented Oct 29, 2024

dimitre commented Oct 29, 2024

roymacdonald commented Oct 29, 2024

artificiel commented Oct 30, 2024

ofTheo commented Oct 30, 2024

roymacdonald commented Oct 30, 2024

roymacdonald commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre Nov 2, 2024

roymacdonald Nov 2, 2024

dimitre Nov 2, 2024

dimitre commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre commented Nov 2, 2024

Addons parsing improvement (dont merge yet) #578

Addons parsing improvement (dont merge yet) #578

Conversation

roymacdonald commented Sep 25, 2024

ofTheo commented Oct 3, 2024

roymacdonald commented Oct 29, 2024

dimitre commented Oct 29, 2024

roymacdonald commented Oct 29, 2024

artificiel commented Oct 30, 2024

ofTheo commented Oct 30, 2024

roymacdonald commented Oct 30, 2024

roymacdonald commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre Nov 2, 2024

Choose a reason for hiding this comment

roymacdonald Nov 2, 2024

Choose a reason for hiding this comment

dimitre Nov 2, 2024

Choose a reason for hiding this comment

dimitre commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre commented Nov 2, 2024

roymacdonald commented Nov 2, 2024

dimitre commented Nov 2, 2024