Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move GLib, json-glib backports to their own translation units #5410

Merged
merged 9 commits into from May 17, 2023

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented May 15, 2023

  • common: Move direct backports from GLib to a new translation unit

    flatpak-utils.c is getting quite large, and is a mixture of code with
    and without non-GLib dependencies, making it inconvenient to reuse in
    other projects (like Steam's pressure-vessel) or link into Flatpak
    services that don't need all of our dependencies (json-glib, appstream,
    ostree). One obvious piece of low-hanging fruit for reducing the size
    of this file is to move the GLib backports into their own
    translation unit.

    Sort them by GLib version, so that when we increase our GLib
    dependency it's easy to delete the ones that are no longer applicable.

    No functional changes intended in this commit.

  • glib-backports: Make g_key_file_save_to_file match the GLib implementation

    This just adds some assertions, no functional changes (assuming we're
    calling it correctly).

  • glib-backports: Add a note that a couple of functions aren't backports

    We can't backport GLib's implementations of these, because they make use
    of GHashTable/GPtrArray internals. Instead, we have a reimplementation
    of the same API, which accepts the cost of some redundant work as a
    reasonable price to pay for backwards-compatibility.

  • glib-backports: Document which version we backported

  • glib-backports: Resync ISO8601 parsing with GLib 2.76.2

    • 3384ed3f "Fixing signedness warnings in glib/gdatetime.c"
    • faa1d63c "glib: Fix various compiler warnings when compiling with G_DISABLE_ASSERT"
    • 4ddabfc6 "gdatetime: Avoid an assertion failure when parsing some ISO 8601 dates"
    • b4eaac58 "gdatetime: Handle leap seconds in ISO8601 dates"
    • f9d0135a "gdatetime: Port to use new g_time_zone_new_identifier() constructor"
      (partially reverted here)
    • c3805d74 "gdatetime: Disallow NAN as a number of seconds in a GDateTime"
    • 5d7f4b8f "gdatetime: Remove floating point from seconds parsing code"
    • d5580edf "Fix non-initialized variable in glib/gdatetime.c"
  • glib-backports: Resync g_get_language_names_with_category with GLib 2.76.2

    • 0e7bf99e "Use "e" mode flag in fopen () calls for race-free setting of the close-on-exec flag"
    • Add #ifndef G_OS_WIN32, which is irrelevant for Flatpak but keeps the
      code textually equivalent to GLib's
  • glib-backports: Move flatpak_utils_ascii_string_to_unsigned to here

    Currently this is used unconditionally, even if GLib is new enough.
    That will be changed in a subsequent commit; no functional change
    intended in this one.

  • glib-backports: Use g_ascii_string_to_unsigned if GLib is new enough

    Use the real GLib function if we can, and resync the backport with the
    version in GLib 2.76.2: use a compatibility replacement for
    G_NUMBER_PARSER_ERROR so that it can be textually identical to the
    version in GLib, and revert Flatpak changes to the whitespace.

    The only functional change is that if the function fails, we'll raise
    G_NUMBER_PARSER_ERROR_INVALID if GLib is new enough.

  • common: Move json-glib backports to their own file

    There is currently no source for this one, only a header.

@smcv
Copy link
Collaborator Author

smcv commented May 15, 2023

ERROR: testcommon - Bail out! flatpak:ERROR:../tests/testcommon.c:556:test_string_to_unsigned: assertion failed (error == (g-io-error-quark, 13)): ?-1? is not an unsigned number (g-number-parser-error-quark, 0)

Hmm, maybe our code that claims to be a straight copy/paste from GLib is not, in fact, a straight copy/paste from GLib? :'-(

@smcv smcv marked this pull request as draft May 15, 2023 16:37
smcv added 3 commits May 16, 2023 11:55
flatpak-utils.c is getting quite large, and is a mixture of code with
and without non-GLib dependencies, making it inconvenient to reuse in
other projects (like Steam's pressure-vessel) or link into Flatpak
services that don't need all of our dependencies (json-glib, appstream,
ostree). One obvious piece of low-hanging fruit for reducing the size
of this file is to move the GLib backports into their own
translation unit.

Sort them by GLib version, so that when we increase our GLib
dependency it's easy to delete the ones that are no longer applicable.

No functional changes intended in this commit.

Signed-off-by: Simon McVittie <[email protected]>
…ation

This just adds some assertions, no functional changes (assuming we're
calling it correctly).

Signed-off-by: Simon McVittie <[email protected]>
We can't backport GLib's implementations of these, because they make use
of GHashTable/GPtrArray internals. Instead, we have a reimplementation
of the same API, which accepts the cost of some redundant work as a
reasonable price to pay for backwards-compatibility.

Signed-off-by: Simon McVittie <[email protected]>
smcv added 4 commits May 16, 2023 12:43
- 3384ed3f "Fixing signedness warnings in glib/gdatetime.c"
- faa1d63c "glib: Fix various compiler warnings when compiling with G_DISABLE_ASSERT"
- 4ddabfc6 "gdatetime: Avoid an assertion failure when parsing some ISO 8601 dates"
- b4eaac58 "gdatetime: Handle leap seconds in ISO8601 dates"
- f9d0135a "gdatetime: Port to use new g_time_zone_new_identifier() constructor"
  (partially reverted here)
- c3805d74 "gdatetime: Disallow NAN as a number of seconds in a GDateTime"
- 5d7f4b8f "gdatetime: Remove floating point from seconds parsing code"
- d5580edf "Fix non-initialized variable in glib/gdatetime.c"
….76.2

- 0e7bf99e "Use "e" mode flag in fopen () calls for race-free setting of the close-on-exec flag"
- Add #ifndef G_OS_WIN32, which is irrelevant for Flatpak but keeps the
  code textually equivalent to GLib's

Signed-off-by: Simon McVittie <[email protected]>
Currently this is used unconditionally, even if GLib is new enough.
That will be changed in a subsequent commit; no functional change
intended in this one.

Signed-off-by: Simon McVittie <[email protected]>
@smcv
Copy link
Collaborator Author

smcv commented May 16, 2023

Hmm, maybe our code that claims to be a straight copy/paste from GLib is not, in fact, a straight copy/paste from GLib? :'-(

No, I was misreading the assertion-failure message: the difference is just that it's raising a different error now.

@smcv smcv marked this pull request as ready for review May 16, 2023 11:45
smcv added 2 commits May 16, 2023 13:01
Use the real GLib function if we can, and resync the backport with the
version in GLib 2.76.2: use a compatibility replacement for
G_NUMBER_PARSER_ERROR so that it can be textually identical to the
version in GLib, and revert Flatpak changes to the whitespace.

The only functional change is that if the function fails, we'll raise
G_NUMBER_PARSER_ERROR_INVALID if GLib is new enough.

Signed-off-by: Simon McVittie <[email protected]>
There is currently no source for this one, only a header.

Signed-off-by: Simon McVittie <[email protected]>
@smcv smcv merged commit d9a3f34 into flatpak:main May 17, 2023
9 checks passed
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.

None yet

2 participants