Created attachment 331316 [details] aseprite-0.9.5.ebuild Animated sprite editor & pixel art tool. This is also available in the overlay "TomWij". I'm up for becoming a proxy maintainer for this package if that's possible. Feedback is welcome...
Created attachment 331318 [details, diff] files/aseprite-0.9.5-as-needed.patch Patch to make the default Gentoo LDFLAGS work.
*** Bug 86673 has been marked as a duplicate of this bug. ***
In the ebuild there still is a > # LIBLOADPNG still missing. present as further testing needs to show whether this is still needed, or already provided through some other dependency; although I have found aseprite to work quite well and have tested it extensively.
Hi A few comments I believe cmake-utils eclass can make use of the PATCHES=() array so instead of implementing your own src_prepare in order to apply the patch you can define PATCHES=( ${FILESDIR}/aseprite-0.9.5-as-needed.patch ) in global scope and drop src_prepare all together. Also KEYWORDS must be sorted alphabetically
Created attachment 331368 [details] aseprite-0.9.5.ebuild Thanks for the PATCHES variable, haven't seen that documented anywhere yet, it's definitely handy. As for the keywords, there doesn't seem to be a hard policy maintaining it (discovered that by asking on chat where I can add checks to repoman) but that I could use `ekeyword` to keep them in order, will do that for now and might add checks for things like these to my local repoman just for readability purposes. Changes for all feedback have been applied. TODO: It appears that the "default fullscreen" doesn't work properly and hence it starts extremely small (even without that define present), there is a --resolution WxH command-line parameter to set its resolution; it appears that resets as the package is emerged again. I need to see whether this is upstream behavior and figure out a fix for this, or perhaps print out instructions for now...
are we ok to commit it here? The ebuild looks ok ( i will give it another go before I commit it). Did you send the asneeded patch upstream?
(In reply to comment #6) > are we ok to commit it here? The ebuild looks ok ( i will give it another go > before I commit it). Yes, should be okay to commit; two notes: Note 1: I haven't yet figured out whether the library "# LIBLOADPNG still missing" is actually needed, it compiled fine without it so it might be that it's no longer an actual dependency; perhaps grepping the entire source might reveal that. Note 2: "-D FULLSCREEN_PLATFORM=ON" doesn't work reliable to me. > Did you send the asneeded patch upstream? No, I didn't do that yet, but I'll do that this weekend.
would it be possible to add more useflags to make all this configure switches optional?
> would it be possible to add more useflags to make all this configure switches optional? Inspecting this, I don't think there is that much optional to this. Let's see... > option(USE_STATIC_LIBC "Use static version of C and C++ runtimes" off) Would I need to add an USE flag "static-libs" for this? > option(USE_SHARED_... "Use your installed copy of ..." off) These just determine whether to use a library provided by the system or to build a copy of the library that is packed along, I don't think it would make sense to provide USE flags to individually turn the shared option on/off as the Portage libraries seem to suffice. > option(ENABLE_MEMLEAK "Enable memory-leaks detector (only for developers)" off) Shouldn't be necessary towards the user, but could be provided as an USE flag "memleak" to make it easier for developers. > option(ENABLE_UPDATER "Enable automatic check for updates" on) We certainly don't want automatic updates outside of Portage, this one is "on" by default so we need to explicitly disable this. > option(FULLSCREEN_PLATFORM "Enable fullscreen by default" off) I haven't seen this option result in a remarkable change of behaviour and don't think it should be an USE flag; we should have a proper default and point the user towards a configuration setting they can change, or parameter they can pass. --------- I have sent the asneeded patch upstream. As a summary, I still need to apply the following changes: - Provide a "memleak" USE flag to toggle the memory-leak detector, disabled by default. - Make sure the updater is disabled, provide no USE flag for this. - Enable fullscreen behavior by default and warn the user through elog how to change it. - Inspect LIBLOADPNG more closely, play around with PNG related features (save and load one). - Act on your answer to below question. So, what do you think regarding to the static-libs USE flag and the individual shared USE flags?
(In reply to comment #9) > So, what do you think regarding to the static-libs USE flag and the > individual shared USE flags? USE="static-libs" is good. As for shared libs - generally you should always use them and not bundled copies, so use flag is probably unneeded, unless bundled libraries are heavily patched and not compatible with those, which is in main tree.
- does virtual/jpeg not work? - please order the dependencies alphabetically - libpng is probably missing a slot (media-libs/libpng:0) unless it works a) with both and b) without headers (since media-libs/libpng:1.2 does not provide any headers and is only available for backwardscompatibility) - afais USE_STATIC_LIBC does nothing
Created attachment 333518 [details] aseprite-0.9.5.ebuild Updated the ebuild with the various suggestions, works without the USE flags. USE=memleak fails with: > /var/tmp/portage/dev-games/aseprite-0.9.5/temp/ccE4cuuz.ltrans19.ltrans.o: In function `ui::LinkLabel::onProcessMessage(ui::Message*)': > /var/tmp/portage/dev-games/aseprite-0.9.5/work/aseprite-0.9.5/src/ui/link_label.cpp:57: undefined reference to `Launcher::openUrl(std::string const&)' > collect2: error: ld returned 1 exit status Do I need to report this upstream? USE=static-libs works.
Created attachment 333520 [details] metadata.xml Contains a description of the package and explanation of the USE flags (copied from CMakeLists.txt); depending on whether this will go through proxy maintenance or recruitment, the maintainer section will probably still need to be updated.
Can we add loadpng (http://tjaden.strangesoft.net/loadpng/) to the tree as well? I don't have this library on my system and can successfully load and save PNG so I don't see what this library is used for, dropping it as a dependency doesn't feel like a clean approach though... I'll ask the upstream developer about this.
afaiu "static-libs" is the wrong useflag here, see the description: static - !!do not set this during bootstrap!! Causes binaries to be statically linked instead of dynamically static-libs - Build static libraries
The as-needed patch has since been applied upstream, see https://github.com/dacap/aseprite/commit/8fb637c8a1dc0091db54f0297940dc42a66ca498 I will apply the latest changes tomorrow and remove the patch in the ebuild by 0.9.6, and perhaps also look into creating a -9999 version. The upstream developer David C. has also told me that > PS: About 'loadpng', it's necessary to load/save palettes in png format. so I'll look into adding that library to Portage as well.
Created attachment 334168 [details] aseprite-0.9.5.ebuild Changed static-libs into static as suggested, will do a -9999 version in a few hours from now.
"-memleak" is pointless, just do "memleak" order the PATCHES array after all dependencies ${FILESDIR} needs to be quoted you don't install any docs, but there are several... in the root dir and in the "docs" subdir as well add x11-libs/libX11 to dependencies are you sure we don't need any useflags for media-libs/allegro? A few tests indicate no, but I haven't done many runtime tests you should provide a debug useflag. The definitions are non-standard so in order to avoid using debug build type you can use flag-o-matic.eclass and do: use debug && append-cppflags -DDEBUGMODE -D_DEBUG it will still add -DNDEBUG if debug is enabled because of some build type checks that don't work nicely with our eclass. I propose a quick sed: sed -i -e '/DNDEBUG/d' CMakeLists.txt || die this way we get "-DDEBUGMODE -D_DEBUG" on USE="debug" and "-DNDEBUG" on USE="-debug" which is the intended behavior from reading the build system order IUSE alphabetically
Created attachment 335082 [details] aseprite-0.9.5.ebuild Added "flag-o-matic" inherit, added FTL license which appears to be used within the Aseprite and not in one of its dependencies (double checked the licenses), added "debug" and "doc" USE flags, adjusted Allegro dependency to need [X,png] as tested, added x11-libs/libX11 dependency, moved PATCHES to after (R)DEPEND, added src_prepare to respect the USE flags, added debug and docs. Some of this is untested as Aseprite currently fails to compile whatever I try (even with the USE flags for which it worked, so I broke something in the process). [31m[1mLinking CXX executable grid_ui_unittest [0mcd /var/tmp/portage/dev-games/aseprite-0.9.5/work/aseprite-0.9.5_build/src && /usr/bin/cmake -E cmake_link_script CMakeFiles/grid_ui_unittest.dir/link.txt --verbose=1 /usr/lib64/ccache/bin/x86_64-pc-linux-gnu-g++ -O2 -pipe -march=corei7 -ggdb -flto -w -fno-lto -Wl,-O1 -Wl,--as-needed -flto -fno-lto -L/usr/lib64 -lalleg CMakeFiles/grid_ui_unittest.dir/ui/grid_ui_unittest.cpp.o -o grid_ui_unittest -rdynamic -lgtest ../lib/libui-lib.a ../lib/libshe.a ../lib/libgfx-lib.a ../lib/libbase-lib.a -ljpeg -lz -lpng -lz -lgif -ltinyxml -lloadpng -lpthread -lpng -lgif -ltinyxml -lloadpng -lpthread /usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld: ../lib/libui-lib.a(theme.cpp.o): undefined reference to symbol 'getg' /usr/lib/gcc/x86_64-pc-linux-gnu[0m/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld: note: 'getg' is defined in DSO /usr/lib64/liballeg.so so try adding it to the linker command line /usr/lib64/liballeg.so: could not read symbols: Invalid operation collect2: error: ld returned 1 exit status
Created attachment 335084 [details] aseprite-0.9.5.ebuild Nevermind that compilation error, that was because the as-needed patch was no longer applied since I had introduced a src_prepare phase and didn't call the cmake specific installation functions. Upon closer inspection I have reworked the documentation installation to just use the DOCS array, which makes the ebuild more clean.
s/src_prepare/src_install/
Created attachment 335748 [details] aseprite-9999.ebuild
Created attachment 338168 [details] metadata.xml Corrected static-libs to static and clarified its description. Metadata reflects new e-mail.
Created attachment 338170 [details] aseprite-0.9.5.ebuild Changed year to 2013. Removed -* from KEYWORDS. Added support for FEATURES="test", remove tests for FEATURES="-test". Corrected static-libs to static. DOCS has been spread over multiple lines. Removed "Full HD" from pkg_postinst example.
Created attachment 338172 [details] aseprite-9999.ebuild Same changes as above, removed SRC_URI and removed -* from KEYWORDS.
Created attachment 338174 [details] aseprite-0.9.5.ebuild Made gtest dependency depend on whether FEATURES="test" is set. Made sure all sed commands use die when they fail.
Created attachment 338176 [details] aseprite-9999.ebuild See previous comment.
+*aseprite-0.9.5 (07 Feb 2013) +*aseprite-9999 (07 Feb 2013) + + 07 Feb 2013; Tom Wijsman <TomWij@gentoo.org> +aseprite-0.9.5.ebuild, + +aseprite-9999.ebuild, +files/aseprite-0.9.5-as-needed.patch, +metadata.xml: + New ebuild for aseprite. Thanks to hwoarang, pinkbyte, hasufell, ssuominen and + mr-bones for proof-reading my first contribution. Fixes bug #445814.