Establish C library base template with Meson for GTK4 #1
Conversation
- core XO buddy colors, utility functions - meson for building setup - added tests
| symbol_prefix: 'sugar', | ||
| identifier_prefix: 'Sugar', |
There was a problem hiding this comment.
symbol_prefix should be sugarext and identifier_prefix should be SugarExt.
There was a problem hiding this comment.
I'm also thinking about us adding install_gir as if this was run in sugar live build, which would potentially be the case in the future then that'll be a needed argument.
| * SUGAR_EXT_C_VERSION_HEX: | ||
| * | ||
| * sugar-ext version, encoded as an hexadecimal number, useful for | ||
| * integer comparisons. | ||
| */ | ||
| #define SUGAR_EXT_C_VERSION_HEX \ | ||
| (SUGAR_EXT_C_ENCODE_VERSION (SUGAR_EXT_C_MAJOR_VERSION, SUGAR_EXT_C_MINOR_VERSION, SUGAR_EXT_C_MICRO_VERSION)) |
There was a problem hiding this comment.
This was made using the gnome-builder template.
There was a problem hiding this comment.
I see no need for it, we can drop it now and add it later if we need it.
src/sugar-ext.c
Outdated
There was a problem hiding this comment.
Why would we need Sugar colors to be part of this library? Considering the old implementation had nothing to do with that.
There was a problem hiding this comment.
I just started with it to kickstart, I have made the list of c-object, this was just easier for me to start with and test. Hence the code isn't modular as well. But I just wanted to get the structure right.
https://github.com/sugarlabs/sugar-toolkit-gtk3/blob/master/src/sugar3/graphics/xocolor.py
There was a problem hiding this comment.
I don't see how that's easier to start with when you could've easily just started with sugar-grid which has only glib and gdk as it's dependencies and would probably not need any rewrite.
What you did was a write of something you won't need at any point in time.
Could you share this list that you made?
There was a problem hiding this comment.
- Here Use as-is means if we decide to keep it we use it or it's newer version or alternatives. Port meaning we would have to change stuff, and whenever I have mentioned GTK4 we will use the new API directly rather than implementing from scratch.
C Objects Migration List
| Object | Dependencies | Purpose | Port/Use GTK4 |
|---|---|---|---|
| sugar-grid.c/h | GLib + GDK | Grid-based layout calculations | Port |
| sugar-fatattr.c/h | Pure C, sys headers | FAT filesystem attribute utilities | Use as-is |
| acme-volume.c/h | GLib, ALSA | Audio volume control | Use as-is |
| acme-volume-alsa.c/h | GLib, ALSA | ALSA backend for volume control | Use as-is |
| sugar-wm.c/h | GLib, GDK, X11 | Window manager interaction utilities | Port |
| sugar-clipboard.c/h | GLib, GTK3 | Clipboard helper functions | Port (GdkClipboard) |
| eggdesktopfile.c/h | GLib, GTK3, GDK | Desktop file parsing and launching | Port |
| sugar-key-grabber.c/h | GLib, GDK, X11 | Global key binding system | Port (GTK4 shortcuts) |
| sugar-cursor-tracker.c/h | GLib, GDK, X11, XInput2 | Mouse cursor visibility tracking | Port (GTK4 events) |
| sugar-gesture-grabber.c/h | GLib, GDK, X11, XInput2 | Global gesture capture system | Port (GTK4 gestures) |
| sugar-event-controller.c/h | GLib, GTK4 | Base event controller | Port |
| sugar-long-press-controller.c/h | GLib, GTK4 | Long press gesture detection | Port |
| sugar-swipe-controller.c/h | GLib, GTK4 | Swipe gesture detection | Port |
| sugar-touch-controller.c/h | GLib, GTK4 | Touch event handling | Port |
| sugar-zoom-controller.c | GLib, GTK4 | Zoom gesture detection | Port |
| sugar-rotate-controller.c | GLib, GTK4 | Rotation gesture detection | Port |
| eggaccelerators.c/h | GLib, GTK3, GDK, X11 | Keyboard accelerator handling | Port |
| eggsmclient.c/h | GLib | Session management client | Use as-is |
| eggsmclient-xsmp.c/h | GLib, X11, ICE, SM | XSMP session management backend | Use as-is |
| gsm-app.c/h | GLib | Session management application handling | Use as-is |
| gsm-client.c/h | GLib | Session management client base | Use as-is |
| gsm-client-xsmp.c/h | GLib, X11, ICE, SM | XSMP client implementation | Use as-is |
| gsm-session.c/h | GLib | Session management core | Use as-is |
| gsm-xsmp.c/h | GLib, X11, ICE, SM | XSMP protocol implementation | Use as-is |
| "copyright" line and a pointer to where the full notice is found. | ||
|
|
||
| <one line to give the library's name and a brief idea of what it does.> | ||
| Copyright (C) <year> <name of author> |
There was a problem hiding this comment.
You have not followed the instructions at this point. Please read and understand the license. Don't delegate that.
There was a problem hiding this comment.
Right! Will update this right away.
- removed clangd and docs - updated copyright - version change and naming of library updated
- added sugar grid as discussed - version changed to 4.0.0 to represent gtk4. - examples running instructions
| @@ -1,5 +1,5 @@ | |||
| project('sugar-ext', 'c', | |||
| version: '2.0.0', | |||
| version: '4.0.0', | |||
There was a problem hiding this comment.
This isn't actually what we meant when we talked about versioning, seems you misunderstood.
We already talked about keeping the existing version convention, which usually goes with the sugar version released.
| @@ -80,4 +80,5 @@ add_project_arguments(project_c_args, language: 'c') | |||
|
|
|||
| subdir('src') | |||
| subdir('tests') | |||
| subdir('examples') | |||
There was a problem hiding this comment.
Why would we need to include examples in the build?
There was a problem hiding this comment.
Right , I saw GTK4 build and it's optional, and we can do that too, but we would ideally have to keep the subdir as otherwise it just won't compile the examples.
There was a problem hiding this comment.
Keeping the subdir is fine by me, adding it as part of the build is something else entirely.
I don't think it needs to be part of the build before you can run the examples.
src/sugar-grid.c
Outdated
|
|
||
| G_DEFINE_TYPE(SugarGrid, sugar_grid, G_TYPE_OBJECT) | ||
|
|
||
| void sugar_grid_setup(SugarGrid *grid, gint width, gint height) { |
There was a problem hiding this comment.
I don't see why the existing definition format is being changed, I think that was more readable than this.
Same goes for every function here.
There was a problem hiding this comment.
It's not me it was the clangd formatter, I can keep the same as old one and turn off my formatter.
There was a problem hiding this comment.
That'll be great, the old format was more readable and I don't see why we need to change it.
|
Reviewed 0bd8ef9. |
AHHH, should I just remove them then? |
I am asking this because I actually already added some sugar-audio.h and .c which I want to further add, maybe after this PR is done with the pipewire and pulseaudio integration porting the old acnevolume files |
Removing them makes sense as they'll not be used here.
New files which are replacements of what's in existence are okay, don't forget to write a comprehensive commit message with as much details as needed. |
- removed xo color as this will be again done in python library - brought back original formatting - updated build systems
Port the FAT filesystem attribute utilities to work with GTK4 build
system and updated GLib patterns.
- Update sugar-fatattr.{c,h} for modern GLib APIs
- Maintain compatibility with FAT filesystem operations
- added comprehensive tests and examples
- added tests and examples to properly run and test the long press controllers - I request to check NOTE: whereever written , it has some info for testing and implementation - updated glib type on fileattr as well - examples display a simple window and work on wayland as well rebase
2025-06-18.16-15-20.mp4 |
|
If there's something we need to do it in this PR let's finish it quickly, it will get weird if we add a lot of files in the Establish PR, and it would be easier for you as well to test if there were separate PRs. |
There was a problem hiding this comment.
Your commit message says "removed xo color as this will be again done in python library" but that's not true as it already exists so there's no doing again.
If you can, avoid committing mode changes to files as the reasons you have for doing so are mostly to text locally.
There was a problem hiding this comment.
If they're no changes to the original file which it looks like that's the case, then the copyright for the original author should be retained and it shouldn't have you as the original author cos you're not.
If you've changed some things then retain the original author copyright and include yours too.
There was a problem hiding this comment.
It's completely different from the original and it's not just setting the hidden fat attribute like last time. I am not sure how this works and so I changed the file name and everything about it .
There was a problem hiding this comment.
my question is why isn't it just doing what it used to do before, what's the need for all the new things?
right, what should that commit be ? 🤔 Should I just write removed xo colors? |
There was a problem hiding this comment.
After looking at sugar-file-attributes.c and your commit message, I don't see why there's a need to use the modern GLib API as that's pretty much what made the changes this much.
The old implementation never used GLib and it worked fine, I don't see why we need to change it now if it still works and nothing has changed.
The old implementation was supposed to be called when a datastore entry is copied to an external device, and it basically set the hidden attributes of that file and I don't see why the implementation needs to change as your commit message doesn't say anything about why this is happening, it just tells us what's happening.
@quozl thoughts?
| if (!value) { | ||
| removexattr(path, name); | ||
| return TRUE; | ||
| } |
There was a problem hiding this comment.
Why are you removing something when it's not supposed to exist?
A better entry would've been "remove xo colors as it's not need here and won't be used, it also has been implemented in the toolkit". |
|
External filesystem support has improved in Linux, so it would be best to know if the file attributes part of the API is still needed. If it isn't needed, then remove the callers now, and target this development for future release of toolkit or Sugar shell. If I recall correctly this part of the design was so that filesystems without metadata support could be used with Sugar. Also appreciate that many people worked at once to produce the original design, and just three people working now can't hope to reproduce the complexity of that work, so it may be necessary to try and fail several times. |
Judging by this, would I be right in assuming this was written to support USB drives and SD cards for the XO as they often use FAT32/exFAT which is pretty much relevant today because very little has changed in the filesystems these devices use.
I agree, I don't think we need to change the existing implementation without having to test it and see that it needs to change. |
On using the modern GLib API:
❯ ./builddir/examples/sugar_file_attributes_example
Sugar File Attributes Example
=============================
Created temporary file: /tmp/sugar_demo_UIMM82.txt
1. Setting basic attributes...
File Attributes:
================
File: /tmp/sugar_demo_UIMM82.txt
Title: My Drawing
Description: A beautiful drawing I made
Tags: art,drawing,creative
Created by: Paint
Created: 2025-06-20 10:49:35
Modified: 1970-01-01 05:59:10
Preview: (none)
2. Using full attribute structure...
File Attributes:
================
File: /tmp/sugar_demo_UIMM82.txt
Title: Updated Drawing
Description: An updated version with more details
Tags: art,drawing,creative,updated
Created by: Paint
Created: 2025-06-20 10:49:35
Modified: 2025-06-20 10:49:35
Preview: /tmp/preview.png
3. Individual attribute access...
Retrieved title: Updated Drawing
Retrieved description: An updated version with more details
Retrieved tags: art, drawing, creative, updated
4. Simulating activity workflow...
Marked file as created by Write activity.
File Attributes:
================
File: /tmp/sugar_demo_UIMM82.txt
Title: My Story
Description: A story about adventures
Tags: story,writing,adventure
Created by: Write
Created: 2025-06-20 10:49:35
Modified: 2025-06-20 10:49:35
Preview: /tmp/preview.png
Example completed successfully!
Note: Attributes are stored as extended attributes (xattr)
and may not be visible in standard file managers.
❯ ./builddir/tests/test_sugar_file_attributes
TAP version 14
# random seed: R02S0454b80c57b19320e0cfd390e0eecd4c
1..4
# Start of sugar tests
# Start of file-attributes tests
ok 1 /sugar/file-attributes/creation
# GLib-GIO-DEBUG: _g_io_module_get_default: Found default implementation gvfs (GDaemonVfs) for ?gio-vfs?
ok 2 /sugar/file-attributes/basic-operations
ok 3 /sugar/file-attributes/full-structure
ok 4 /sugar/file-attributes/activity-marking
# End of file-attributes tests
# End of sugar testsI have this for every single thing I have ported and will continue to have for everything as well. Testing is probably the most important part I am worried about. |
How does using the API improve maintainability and familiarity with the code base?
From Quozl's comment, if the aim was to support filesystems without metadata support, wouldn't modernizing it defeat that purpose?
I was talking about testing what's already in existence if it needs to change. |
My main idea was to unify the codebase under a single, modern standard.
That's a critical point. My approach relies on the GIO/GVfs backend, which is designed to handle this. On filesystems like FAT32 that lack extended attributes, it should automatically fall back to creating hidden metadata files. So, we get a modern API without losing compatibility. However, you're right that this needs to be confirmed. We can keep this as TODO to test no FAT32 drive later if that's possible?
Ahh I see, my apologies for the lack of understanding. The rewrite wasn't because the old code was broken, but for architectural consistency. Since this PR is about establishing a new GTK4/GLib foundation, I felt it was better to port this module to the new standard rather than mixing old and new styles. |
|
You can test FAT32 immediately using Linux, using a loopback device to map a file as a block device, then format it. Consider also the next step after Sugar shell and toolkit conversion to GTK 4, which is the conversion of each activity to GTK 4. You're only a tiny blip on the overall effort. ;-) |
This is something that should be in your commit message. |
|
I looked again, and now I understand the use of Firstly one of the file attributes FAT exposes is the Judging from the last change in Sugar which uses that - seems to be the only use of it - we don't necessarily need to have it set because we're assuming Sugar is run on Linux which automatically hides files beginning with a dot, which in this case is In essence, if we decide to keep using Your change creates and sets attributes which was never the reason the old implementation existed in the first place. Although the |
Summary
.gitignorefile, andCOPYINGwith the LGPL-2.1 license.Questions:
Testing
meson test -C builddirHelpers
{ "configurations": [ { "name": "Linux", "includePath": [ "${workspaceFolder}/**", "${workspaceFolder}/builddir", "/usr/include/gtk-4.0", "/usr/include/pango-1.0", "/usr/include/fribidi", "/usr/include/harfbuzz", "/usr/include/gdk-pixbuf-2.0", "/usr/include/cairo", "/usr/include/freetype2", "/usr/include/libpng16", "/usr/include/pixman-1", "/usr/include/graphene-1.0", "/usr/lib/graphene-1.0/include", "/usr/include/glib-2.0", "/usr/lib/glib-2.0/include", "/usr/include/libmount", "/usr/include/blkid", "/usr/include/sysprof-6" ], "defines": [ "_FILE_OFFSET_BITS=64" ], "cStandard": "gnu11", "cppStandard": "c++17", "intelliSenseMode": "linux-gcc-x64", "compilerPath": "/usr/bin/gcc", "compileCommands": "${workspaceFolder}/builddir/compile_commands.json" } ], "version": 4 }