Especially as our code does not actually leak as far as we can see. It looks
like librsvg might not play well with -fsanitize=address (possibly having real
leaks or false positives).
Cf. the previous commit: colorsvg2png has a memory leak in librsvg (so we can't
fix it easily). In any case, it's just a one-time-use tool, we don't really need
to focus on its memory bugs as long as it does its job to make icons.
Actually even with this, b_sanitize=address still detects a memory leak. After
some testing, it seems that just creating a RsvgHandle, then freeing it
immediately is enough to leak some data, which means the leak is in librsvg.
This will allow to also check the list of runtime builds. We could see an
example in a report (#8993) where someone had the latest flatpak build of GIMP
but an older build of the runtime flatpak. So they had a bug because of a
dependency which got updated since then.
- Setting an exec_dir variable is an error. As meson docs says, if
relative, it is installed relatively to prefix anyway: "If this is a
relative path, it is assumed to be relative to the prefix."
On the other hand, it would make problems if someone tried to set an
absolute bindir.
Moreover it is a lot clearer now. When we want to install in the
binary directory unconditionally, then get_option('bindir') is the
meson way, hence the way to go.
- On the other hand, the `gimp-debug-tool` is installed either in bindir
for Windows or macOS and libexecdir for all other platforms, at least
that's how it's set in the autotools build. So let's keep both builds
consistent.
- Make a hopefully clearer description for enable-default-bin option.
Let's clarify this is just about creating unversionning links pointing
to versionned files.
- Adding an item in the "Optional Features" part of the summary listing
during meson configure, for better discovery.
For the ".exe" extension on Windows, I wished we had an $(EXEEXT)
equivalent on meson rather than trying to set it ourselves (are there
other platforms where we must set a different extension?). But I could
not find any.
Our meson build system was not properly building the enums.c file,
because they are versionned.
I did a similar trick as what I did for the pdbgen, which is that I used
a wrapper script around the existing perl script, which sets proper
options and generate a stamp file in the end (which is considered by
meson as the actual custom target, not the C file since it is generated
in the source dir).
The most important part is that the stamp file is a generated header
source (not just a random text file) which is **included** by the
generated C file. This is what will force meson to regenerate the C file
if the header is updated, **then** build using this new version, not use
an outdated versionned version (which would make for hard to diagnose
bugs), through the indirection of the intermediate stamp header.
See #4201.
See also: https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080742592
Displaying it all the time when we fail the first attempt is confusing
as it is expected to fail in the meson build (since build libraries are
in different folders). Instead only output the error message when both
known paths failed, i.e. when we fail the script (and write down both
attempted path in the error message).
The check script now takes into account both the autotools and meson
file hierarchy (in autotools, built libs are in .libs/ subdirs).
Also it now properly fails on missing lib.
Since we are pre-processing anyway the AppStream metadata file (because
appstream-glib doesn't pass unknown XML attributes, cf. a previous
commit), it does feel silly now to continue loading the file at runtime
too. Let's just pre-process more data into the constructed C files, i.e.
get the introduction paragraphs and the change items too.
The only other remaining advantage of appstream-glib was that it was
handling the localization but since we also have these localized strings
in our gettext files, we can as well translate with gettext as any other
strings and it works just fine.
It will also get rid of any packaging issue, forgetting to package the
metadata file (as we had on the Windows installer, and still have on the
macOS package). Now it will just always work because data is internal.
The idea is to add some "demo" attribute to a list item inside the
<release> tag, since we already decided that (for now at least) we'd
keep a strict "intro + list" logics, as we did until now.
This demo attribute uses an internal format to specify successive
widgets to blink (like a demo path towards a feature). For now, what it
allows is:
* raise the toolbox, select a tool and blink the tool button.
* raise a dockable, blink any widgets in there.
Now it is still limited and needs to evolve. In particular:
* What happens if the blinked tool button was explicitly removed from
Preferences? Should we re-add it for the demo? And once done, should
we remove it again? Then should we select back the tool previously
selected?
* What happens if the dockable widget is not visible? Should we allow
changing the settings to be able to demo correctly the new/changed
settings? Should it be temporary? If temporary, it can be annoying as
you'd still have to look attentively the demo to find back the path to
the new settings. If not temporary, some people may dislike we touch
their settings.
* What if docks are hidden? Should we unhide them, then hide them back
after demo time?
Also regarding the implementation: originally I wanted to just grab the
demo attribute directly from the AppStream metadata file, but I realized
that appstream-glib cleans out unknown attribute from the XML. I could
then simply parse the file with a generic XML parser, but I found
simpler to pre-parse it into a header built within GIMP. I still use
appstream-glib at runtime as it takes care of localization for us
(though in the same time, we also have the localization in the main po
files, so maybe we could just embed the release note strings as well).
See appstream-glib report: https://github.com/hughsie/appstream-glib/issues/431
Makes more sense and I am trying to make the devel-docs more readable
(which includes less crowded, especially with scripts which are not
really docs).
Ok that was a bit of a mess with the 4 build cases (combinations of
meson, autotools, vector and raster icons). I *think* this is now OK.
Basically we still need to build the colorsvg2png tool even when
installing vector icons, just for the purpose of the 2 icons
dialog-question and gimp-wilber-eek which we compile into GLib resources
from PNG images.
Also it looks like I completely forgot to add the subdir meson.build in
icons/Color/.
Even though we only generate makefiles, we actually want to make meson
aware of the list change because it continues using the file list from
the previous configuration. We do this by "touching" the meson.build
files, i.e. editing their access time.
Also rebuild the icon-list as there are small changes forgotten.
Some points to note:
- gimp-web and gimp-toilet-paper had some usage elsewhere so I also left
them in 2 of the more generic size files.
- I remove gimp-floppy altogether as it's used nowhere in any of our
code or data files.
There are some clear and obvious groups in the icons. For instance, the
Preferences icons are one of them. Looking up the code, we only use them
in 16px (in Preferences side menu) and 48px (in Preferences page
headers). Until now, we were storing in other unrelated size (22px) and
also the lists per-sizes were not consistent. Some icons were missing
here, other there.
With this new organization, the Preferences icons are listed in a single
file, ensuring usage and contents consistency. Also it allows to install
them only for the needed sizes (note that it is possible that they might
be needed in different size, for instance with custom themes; but we
can't just randomly distribute icons in all sizes; or to be more
accurate, this is exactly why we encourage rather the SVG/scalable
icons, so if some people explicitly go for raster icons, they also get
the drawbacks which come with).
Last note: it may be possible that some icons end up in different
"semantic" icon group. This is not a problem with this new organization
as my scripts handle duplicates gracefully.
Build-time tool, which basically just rasterize SVG images (it doesn't
do anything special like gtk-encode-symbolic-svg which creates special
PNG for GTK to recolor them).
It looks like I had this prepared since 2018 according to file header,
but I just never finished doing it! :P
Basically now PNG icons of the Color icon themes do not need anymore to
be committed in the repository. They will be generated from the SVG
icons.
Also adding a missing icon from the 16px list (the Playground icon for
Preferences dialog was needed in 16x16 as well, yet missing).
It may actually be meaningful to have redundant vector icons at specific
size, for instance when you want sligthly different designs (e.g. more
details at bigger size). But here looking at our 24x24 vector icons, it
doesn't look like it at all. It's mostly the exact same icons,
duplicated (with some forgotten).
Makes no sense here. So let's simply install the scalable/ icons for all
size, and that's it.
Just so that someone who happens to look in there is not tempted to
modify these files directly and get some instructions.
The comment is inspired from other similarly generated Makefile.am (i.e.
in plug-ins/commons/).
… listing.
This way, both the Color and Symbolic icon themes, both on meson and
autotools build systems all share the exact same list of icons.
I think there are still improvements to be done on what we list and
chosen sizes and icon categories. But this step is about just getting on
the same state as we currently are, simply with shared listings. Once we
are there, we can go forward.
Sometimes we want to make quick tests on old versions of GIMP.
Rebuilding from source is definitely still an option, yet with flatpak,
we have many past builds available easily to us (at time of writing: 19
stable builds, 12 dev point-release builds and at least 3 nightlies —
though I seem to have issues with signatures on gnome-nightly right now,
so maybe there are more!).
There are some command lines needed to check the build history, then to
install a specific build, which I explained in developer docs (see
devel-docs/debugging-tips.txt, section "Testing older GIMP versions").
Yet it's clearly cumbersome and slow so I wrote this script today to
automatize the process a bit.
Running simply this command will list all available releases on the
Flathub repository (adding --beta or --nightly will list the development
releases and nightly builds instead):
$ tools/flatpak-releases
The listing will contain a topic describing the build as well as the
date, all this prefixed by a number. For instance, this is an excerpt of
the output for the dev releases:
$ tools/flatpak-releases --beta
0: Update dependencies (127a0fa7) [2022-01-13 16:59:43 +0000]
1: Issue #106: File->Create->From Screenshot not working. (9c831b14) [2021-12-14 21:46:52 +0000]
2: Release GIMP 2.99.8. (908bf5b0) [2021-10-20 20:29:00 +0000]
3: Release GIMP 2.99.6. (e04355dd) [2021-04-26 14:08:32 +0000]
…
The last build updates dependencies, the previous one fixes some
specific issue and the 2 previous ones are point releases.
Now say I needed to test/compare some behavior with how it was in 2.99.6
(e.g. to verify a regression), I would then run:
$ tools/flatpak-releases --beta -3
This would install this specific dev build number 3. In just 2
easy-to-remember commands and a few seconds, we can therefore list and
install specific Flatpak builds.
On Windows, the macros COPY, REMOVE and REMOVE_DIR are allocated
strings, unlike on other platforms, so the way we use these returned
values, they are never freed.
Instead make win32_command() keep a copy of the allocated string as
static to the function and free it there at each next call (the returned
value being type-casted to (const gchar *)). It will still leak the last
string, but anyway gimptool is short-lived.
Also it should silence static analyzers.
It now uses rsvg_handle_get_geometry_for_layer() instead of
rsvg_handle_get_(position|dimensions)_sub() which have been deprecated
in favor of the new function which returns accurate (double) positioning
and size.
Back when I first made this build tool, librsvg had several blocking
bugs to make icon extraction work properly. This seems to improve
nicely.
Nevertheless I still don't build this tool because after testing, more
bugs remain and icon extraction is still not right. Just updating the
code to undeprecate it but leave the build commented out for now. ;-(
See librsvg#134, librsvg#128 and librsvg#250 for bug reports following
these issues.
Add a new performance-log-progressive-coalesce.py tool, which
coalesces partial address maps in progressive performacne logs into
a single global address map, suitable for processing by the rest of
the tools.
Use the new tool as part of the pipeline in performance-log-viewer.
Add a new performance-log-close-tags.py tool, which closes unclosed
tags in incomplete performance logs, allowing the rest of the
perofmance-log tools to process them. This is necessary for
unfinished progressive logs.
Use the new tool as part of the pipeline in performance-log-viewer.
Commit 283ec1da0f previously pushed had some coding style bugs, which
unfortunately couldn't be fixed before pushing because the platform
doesn't allow it and the original contributor is not available lately.
Let's fix these.
Continue installing the list of files to install even if a failure
occured. In particular, I encountered a problem where script-fu was
failing to install because the installed binary was being executed (and
its memory probably mmap-ed). Hence the copy failed with:
> [Errno 26] Text file busy: '/my/prefix/lib64/gimp/2.99/plug-ins/script-fu/script-fu'
Yet trying to reinstall plug-ins while GIMP is being run, hence
script-fu is being executed, is a very common development trick (for me
at least). So instead, when such an error occurs, I simply save it and
output all the exceptions in the end (where these warnings are properly
visible), instead of failing everything.
Of course, ideally the subtarget should not even try to install
script-fu as it is unchanged. This is why I changed shutil.copy() by
shutil.copy2() to install metadata, which include last access timestamp.
Maybe this could be used to decide whether the installed file is already
the last one, thus not retry, though I am actually unsure if this is
enough (on the other hand, the ultimate check of bit-to-bit comparison
may obviously be too much/slow which is counter-productive), which is
why I am not doing this change right now.
Add a new mnemonic-clashes tool, which checks for mnemonic clashes
in menus. This tool can be invoked directly from the shell. It
takes an optional parameter which limits the search to a specific
type of menus, according to their xml tag (in particular, "menu"
for regular menus, and "popup" for popup menus).
We should not rely on the current working directory, as it looks like
meson sets it to be the source directory (even when actually in the
build dir) when running custom target (so it fails).
Instead meson explicitly sets MESON_SOURCE_ROOT and MESON_BUILD_ROOT
environment variables for us. We should use these. This should work in
every cases (whether calling from the source or build dir).
Also removing the special-casing for a _build/ directory. This updated
version is generic and won't assume that you used some generic naming or
direct subdir under the source (even if many people might use this path
and name; I, for one, certainly don't!).
See: https://mesonbuild.com/Run-targets.html
Jehan