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

MicroG should *not* set WITH_GMS = true #358

Closed
infinity0 opened this issue Nov 20, 2022 · 35 comments
Closed

MicroG should *not* set WITH_GMS = true #358

infinity0 opened this issue Nov 20, 2022 · 35 comments

Comments

@infinity0
Copy link

WITH_GMS = true is an internal LineageOS build-time variable which means more than simply "Google services are part of this build". According to LineageOS devs in #lineageos-dev:

<LuK1337> and WITH_GMS is mostly meant to be used by those who know what it's doing
<timschumi> whoever builds with WITH_GMS presumably also bakes in all the other modifications that they need

Internally, LineageOS treats WITH_GMS as implying that the user will never install any further addons, and therefore will not reserve any extra space in the relevant system partitions when it is set.

Because MicroG is built with it set, system partitions have no extra space in them, meaning that addons that need to modify /system fail to do so, resulting in bugs like topjohnwu/Magisk#3820 (comment) and arovlad/bromite-webview-overlay#5.

There is no benefit to setting WITH_GMS from MicroG's perspective; LineageOS official builds do not set it, and GApps addons such as MindTheGapps/OpenGApps do not "switch it on" after being installed, as it is a build-time variable only and is invisible at runtime.

<infinity0> at runtime, i suppose there is no way to directly distinguish if the build was done with WITH_GMS or not, so it doesn't matter from a runtime perspective?
<LuK1337> no

Therefore, please stop building your LineageOS+MicroG images with WITH_GMS, and stop recommending this practice in this repo's documentation, as it prevents addon installers from working with no actual benefit in return.

@infinity0
Copy link
Author

LineageOS does typically reserve quite a lot of space for the regular GApps addons, so you may want to set BOARD_{PRODUCT,SYSTEM,SYSTEM_EXT}IMAGE_PARTITION_RESERVED_SIZE explicitly to something like 50MB.

@TinfoilSubmarine
Copy link

LineageOS does typically reserve quite a lot of space for the regular GApps addons, so you may want to set BOARD_{PRODUCT,SYSTEM,SYSTEM_EXT}IMAGE_PARTITION_RESERVED_SIZE explicitly to something like 50MB.

This is the reason it was originally added, or at least why I reccomended it be added (for blueline specifically).

@infinity0
Copy link
Author

This is the reason it was originally added,

OK that makes a bit of sense but it has the side effect of breaking other addons. I'll leave it to the devs here to discuss the best way forward.

@bananer
Copy link
Contributor

bananer commented Nov 23, 2022

Besides the partition sizes, we're also using WITH_GMS to include the product definition from https://github.com/lineageos4microg/android_vendor_partner_gms

@infinity0
Copy link
Author

infinity0 commented Nov 23, 2022

Are you able to monkey-patch in extra definitions to the LOS+MicroG build? Setting the RESERVED_SIZE variables explicitly would also fix the issue.

Alternatively, LineageOS devs seem to suggest to me that WITH_GMS is not meant to have a stable public meaning, so if you rely on those product definitions, it may be better to copy them into the LOS+MicroG build explicitly, rather than relying on them being set via WITH_GMS.

@Iey4iej3
Copy link

I second that this should be changed. Currently, the free space is extremely limited, and for example, Lygisk is not able to use /system/addon.d for a backup of less than 20MB (I spent a lot of time locating this issue, cf. programminghoch10/Lygisk#17).

@bananer
Copy link
Contributor

bananer commented Jan 3, 2023

If you can find a better way to adjust the partition size so that our system apps fit on all supported devices, I would be very happy to accept a pull request.

@Iey4iej3
Copy link

Iey4iej3 commented Jan 4, 2023

I am a user, but what is wrong to set the reserved size to be, say, 50MB?

@infinity0
Copy link
Author

If you can find a better way to adjust the partition size so that our system apps fit on all supported devices, I would be very happy to accept a pull request.

The fix would be to set BOARD_{PRODUCT,SYSTEM,SYSTEM_EXT}IMAGE_PARTITION_RESERVED_SIZE explicitly to something like 50MB. What is your preferred way to do that with the current build system?

@bananer
Copy link
Contributor

bananer commented Jan 22, 2023

If I interpret the relevant makefiles correctly, it's actually SSI_PARTITIONS_RESERVED_SIZE we should set. Default is around 30MB, which is too small for microG and co, WITH_GMS sets it 1.2GB which is way more than needed here.

Our current way of including the extra packages relies on partner_gms.mk, which will no longer activate if WITH_GMS is not set.

We could return to the old way of including them via CUSTOM_PACKAGES (see this commit and here). A better solution, which would leave CUSTOM_PACKAGES up for other uses and keep the list of packages defined in our vendor_partner_gms repo, is to patch the inherit-product-if-exists line into common.mk with a distinct flag (i.e. WITH_MICROG). We might also rename the repo/path to vendor_partner_microg to be consistent and avoid conflicts.

Maybe this makes enough sense to you to try it out. If I find the time, I might also give it a shot.

@bananer
Copy link
Contributor

bananer commented Jan 22, 2023

If I interpret the relevant makefiles correctly, it's actually SSI_PARTITIONS_RESERVED_SIZE we should set. Default is around 30MB, which is too small for microG and co, WITH_GMS sets it 1.2GB which is way more than needed here.

Disregard that, the files I was looking at are only relevant for a few devices.

Looking through the actual places where e.g. BOARD_SYSTEMIMAGE_PARTITION_RESERVED_SIZE is defined, the value is different from device to device and only conditional on WITH_GMS in a few cases. I doubt that setting it to the same value for all devices will result in usable builds. Might still be worth a try.

@htto
Copy link

htto commented Jan 27, 2023

Geez... took me ages to figure out that this is the issue why I cannot install AuroraServices. There're only 2.5MB on system and 0.9MB on system_ext available, whilst plain lineage provides the 1+GB on system.

Anyhow, since the builds already patch the version in lineage's vendor (config/common.mk or config/version.mk) [1], wouldn't it be possible to just patch away the if in config/partner_gms.mk and not set WITH_GMS [2]? Though, I only looked at lineage 20, where config/partner_gms.mk is included unconditionally in config/common.mk [3], this would probably save you from the problem of the 50MB causing issues on low storage devices.

On the other hand, the relevant thing in config/partner_gms.mk is probably

        ifneq ($(GMS_MAKEFILE),)
            $(call inherit-product, vendor/partner_gms/products/$(GMS_MAKEFILE))
        else
            $(call inherit-product-if-exists, vendor/partner_gms/products/gms.mk)
        endif

Wouldn't it be possible to add that one line from the else [4] (you don't seem to set GMS_MAKEFILE to the bottom of config/common.mk or replace the config/partner_gms.mk include in that file?

Best regards
Heiko

[1] https://github.com/lineageos4microg/docker-lineage-cicd/blob/master/src/build.sh#L206
[2] https://github.com/LineageOS/android_vendor_lineage/blob/lineage-20.0/config/partner_gms.mk
[3] https://github.com/LineageOS/android_vendor_lineage/blob/lineage-20.0/config/common.mk#L218
[4] https://github.com/LineageOS/android_vendor_lineage/blob/lineage-20.0/config/partner_gms.mk#L32

Lanchon added a commit to Lanchon/docker-lineage-cicd that referenced this issue Aug 2, 2023
Introduce 'WITH_MICROG' to replace use of 'WITH_GMS'. Fixes lineageos4microg#358.
@petefoth
Copy link
Contributor

petefoth commented Aug 2, 2023

A search in the LIneageOS repos indicates that this flag is not used only to set partition sizes.

If we decide to change how we use it, then the change would need to be fully tested. To fully test, we would need to check not only that builds for all ~220 devices do not break, but also that the functionality of those devices is not broken in other ways. Clearly that is beyond the capabilities of this project and is not going to happen.

Given that this issue does not affect the core functionality of our ROMs, and only impacts the uses of some system addons for some devices, then I am not inclined to make any changes.

I'm happy to leave this issue open for further discussion, but I won't be rushing to push any PRs

@Lanchon
Copy link

Lanchon commented Aug 2, 2023

If we decide to change how we use it, then the change would need to be fully tested. To fully test, we would need to check not only that builds for all ~220 devices do not break, but also that the functionality of those devices is not broken in other ways.

i totally disagree with this claim.

the builds without WITH_GMS have been fully tested, by none other than the LOS project, its device maintainers, and its users. they have all been tested extensively, contrary to builds with WITH_GMS which have seen little testing in the best case (revealing failures with many addons), and i suppose no testing whatsoever for most devices.

also, as @bananer says here, builds published by los4mg used to be created without WITH_GMS all the way up to december 2021, so even users of los4mg have tested these builds.

also, microg's developers work and test using builds without WITH_GMS, and the LOS team does not use the option either.

but on the contrary, using the option indeed causes issues; which is why we are here.

A search in the LIneageOS repos indicates that this flag is not used only to set partition sizes.

yes, and it's used for stuff that we don't want:

as far as i can see, all the other uses revolve around not preparing extra spaces for addons.

@petefoth

This comment was marked as off-topic.

@Lanchon

This comment was marked as off-topic.

Lanchon added a commit to Lanchon/docker-lineage-cicd that referenced this issue Aug 2, 2023
Introduce 'WITH_MICROG' to replace use of 'WITH_GMS'. Fixes lineageos4microg#358.
@petefoth
Copy link
Contributor

petefoth commented Aug 3, 2023

@infinity0 you write that

OK that makes a bit of sense but it has the side effect of breaking other addons

Please can you specify which addons you know or believe to be are broken, and on which devices? This will give us a better idea of the scope of the problem, and help us decide on how to move forward with this issue .

Thanks

@Iey4iej3
Copy link

Iey4iej3 commented Aug 3, 2023

An example:.Lygisk on gta4xlwifi, cf. programminghoch10/Lygisk#17 (comment)

@petefoth
Copy link
Contributor

petefoth commented Aug 3, 2023

An example:.Lygisk on gta4xlwifi, cf. programminghoch10/Lygisk#17 (comment)

Thanks. Do you remember offhand which los4mg buid the problem occurred in, and whether or not it still occurs for this add-on, on this device? No porblem if not - just trying to gather data :)

@petefoth
Copy link
Contributor

petefoth commented Aug 3, 2023

I have posted in the XDA Forum thread asking for other users' experiences - good and bad - with installing add-ons. Let's see what comes out of that. I will try to maintain the following tables, based on responses here and in the forum thread, and on my own testing:

Failed installs

Add-on Device los4mg build failure reason
Lygisk gta4xlwifi ? No space on system partition

Successful installs

Add-on Device los4mg build
Lygisk z3 18.1 official
Lygisk z3tc 18.1 unofficial
Lygisk lilac 20.0 unofficial

@petefoth
Copy link
Contributor

petefoth commented Aug 3, 2023

I tested installing Lygisk on the devices I have access to. I installed by flashing from recovery, then opening the installed app and letting it do its stuff. No problems with insufficient space, on system partition or elsewhere

Add-on Device los4mg build outcome
Lygisk z3 18.1 official OK App present & behaving normally
Lygisk z3tc 18.1 unofficial OK App present & behaving normally
Lygisk lilac 20.0 unofficial OK App present & behaving normally

Note that the unofficial builds were all built using the WITH_GMS flag, as recommended in the README.md ;)

I also tried installing Magisk on the same devices, using the deprecated 'Rename and flash from recovery` method. No reported insufficient space problems, but the install didn't succeed - no sign of the app after rebooting. I'm not going to start messing with patching images, so I'm not going to count these attempts as valid tests

@Iey4iej3
Copy link

Iey4iej3 commented Aug 3, 2023

No problems with insufficient space, on system partition or elsewhere

The Lygisk installer does not handle the exceptions appropriately, thus it will show "success" even if there is no space. Furthermore, these spaces are not essential to (first) install Magisk, but essential to make root persist after updates. The point of Lygisk (being a fork of Magisk) is to make root persist after updates, and if there is not enough space, after updating (or even after reflashing the current ROM), the system might not boot.

More technical details: in "modern" environments, the recovery can no longer access encrypted /data partition. The strategy of Lygisk is to make a copy of necessary files from /data to addon.d in /system, and when the system is reflashed (e.g. updated), scripts in addon.d are run, and Lygisk installs itself after flashing. If the /system does not have enough space, then the copy in addon.d is incomplete, and thus Lygisk is not correctly installed after flashing the ROM. Some broken copies even make the system unbootable.

@Lanchon
Copy link

Lanchon commented Aug 4, 2023

hi @petefoth,

thanks for getting onto this and doing this investigation. i'll try to help sharing what i know.

but first...


magisk

magisk is a "system-less" mod, meaning it doesn't touch the system partition at all. magisk works by modding the boot ramdisk (typically bundled with the kernel in boot, but stored standalone in init_boot for android 13+) and from there bind-mounting stuff over /system.

the reason for its existence is maintainable rooting of stock firmwares. it used to be that, once you root, you couldn't accept OTAs anymore. now you can easily unroot (just restore the tiny boot or init_boot partition), apply the OTA, and reroot.

magisk may also install the manager app, but this is complimentary. the boot runtime is the important piece, you can install the app from any source later. this is why i think your tests actually succeeded: you didn't get the app, but if you had manually installed it, it would have found the runtime and worked.

btw, i think the app is always installed in /data to remain system-less. in your case, maybe recovery couldn't decrypt and mount userdata, which cause the app to be missing. (though maybe magisk detects some known free roms like lineageos and installs it in system only for those. i doubt it, but i'm no magisk expert.)


back on-topic...

in the beginning system was a regular ext4 partition and the OTA just wrote files to it. then came dm-verity and OTAs had to be block-level: the android build system now produced a filesystem image instead of just files. for this the build system had to know the filesystem type and the size of the system partition for each device.

by upgrading old phones, cyanogenmod stumbled on a roadblock at the time: old devices had small system partitions, but android kept on getting bigger. users countered by installing smaller and smaller gapps, but eventually this became futile and android itself was getting chopped up to fit by device maintainers.

so people started repartitioning their devices (see https://lanchon.github.io/REPIT/ - ahh, the good old days of unsigned partition tables!). this brought on a problem: aftermarket roms like cyanogenmod had system images of stock sizes, but people had physical system partitions that did not match. as a stopgap measure, TWRP added a manual option to grow the filesystem in system partition to match its partition size. but eventually cyanogemod and friends caught on and added the grow as a post-install step in their zips. (this meant that the build system didn't need to know the physical partition size anymore, but it was useful to have it anyway to know when repartitioning ceased to be optional and became mandatory.)

now this is important: this was the status quo for all devices before android 10, and still is for devices that opt to launch with physical system or system_a/b partitions. for all these devices, the size of the filesystem image within the lineage zip is irrelevant, as the bundled install script will grow the filesystem to the size of the physical partition. i didn't look but i am willing to bet that, for all of these devices, the WITH_GMS setting does nothing at all to their OS partition sizes. this is why if you search for WITH_GMS in the lineage repos you'll find just a handful of matches and not hundreds, as you would expect if there were a match for each supported device.

so older devices won't have the no-space-in-system issue, but as time goes by newer devices will. this is why we can't just sit around and ignore this. it also doesn't surprise me that the older devices on which you tested didn't display the issue.

but moving on... next came android 10 and with it the (optional) dynamic partitioning scheme. there's a lot of variation in implementation here: one can choose any partition accessed after the kernel and its ramdisk loads to be 'dynamic', but typically system (or system_a/b), vendor (or vendor_a/b), etc are chosen to be dynamic (and userdata is not).

(btw, dynamic partitioning is orthogonal to seamless a/b upgrading, so you can chose either, or both, or none. but there's yet another scheme since android 11: an optional 'virtual a/b' scheme that cleverly merges these features.)

with dynamic partitioning there is a big physical super partition which acts as a container, and dynamic partitions are created within it via dm-linear. the objective is not having to reserve large spaces for system, system_ext, vendor, odm, product, etc... or their a/b variants, and create those partitions within super during each OTA to fit their exact image sizes.

now you see the problem: the aforementioned grow-after-install step became a no-operation: for devices that have dynamic system partitions (probably all devices that use dynamic partitions) their post install system partitions will have no free space at all!

so in order to support installation of gapps as an addon on those devices, the lineage people had to implement a solution atop AOSP. for dyn-system devices they just added a bunch of extra margin space to the dynamic filesystem image creation steps, but only if gapps are not being included in the build, and they overloaded the WITH_GMS setting for this.

this was a quick dirty hack, and since it's implemented for each device and not centralized, it was a very bad call. my guess: it was probably done by the first maintainer of a dyn-system device on their own device tree, then other maintainers just copied the stuff to their trees, and nobody provided a system-wide solution... and here we are.

why is it a bad solution? if you build without gapps, which is what they do, it sort of works: each maintainer chooses a max gapps size (probably copying from other maintainers), and if gapps ever grows too large, well they are all screwed. but the issue is building WITH_GMS=true: this flag doesn't work as it did before! before, it just added gapps and the post-install step would enlarge the partition and you'd have space for further addons. now WITH_GMS=true produces tight fitting partitions that don't support addons (breaking an important implicit functionality of LOS) and if the lineage people ever use it they'll run into a wall... just as we did.

but a good solution is not trivial. it might look like this:

  • for each partition the user cares about, they specify the preferred and minimum acceptable amounts of free space.
  • for each partition:
    • if the partition is physical:
      • if the build knows its physical size:
        • fail build if free space would be less than minimum.
        • warn if free space would be less than preferred.
    • if the partition is dynamic:
      • add the preferred free space to the image.
  • if a super partition exists and its size is known:
    • fail build if all dynamic partition don't fit into super.
    • warn if all dynamic partition don't fit into super, counting dyn A/B partitions 2 times (or N times, as N can be > 2).

with all that behind...

what can we do?

it's clear to me at least that we can't ignore this: microg users are modders, and we hate not being able to mod!

for non-dyn-system devices (the majority) this is a non-issue: WITH_GMS should not be found in their device repos, given that no space hack is needed and their filesystems will be grown to max during install, and so we can choose whatever we want for that setting and it won't affect the build. and if it does affect it (i think in only 2 google devices this is used to include a widevine DRM blob), we certainly want if OFF.

for dyn-system devices (the minority, but growing):

  • WITH_GMS=true is a horrible choice:
    • it totally limits hackability, which is unacceptable for many users, me included.
  • WITH_GMS=false is an ugly (but not horrible) choice:
    • it reserves an awful lot of free space (given the sheer size of bloatware that gapps have become) that users don't really need.
    • but a lot of space is much better than none at all!
    • and we are talking of fairly new devices, with large super partitions, and this space would go to waste anyway if not used.
    • but there is a risk:
      • though lineageos+exta_space must fit in super, lineageos+mircog+exta_space might not.
      • it is unlikely because microg is so small:
        • but if it happens we'll probably know by user reports... :(
          • same applies to LOS devs, btw.
        • if it ever happens:
          • we talk to the LOS device maintainer, explain the issue, have them accept a patch where:
            • if WITH_MICROG=true, add half the space that was being added for gapps.
          • or we apply a one-line patch to the device build file that changes the added space in system.
          • or we build that device with WITH_GMS=true. (yuck!)
          • or maybe LOS fixes this whole space-on-dyn-system-devices mess and we start using their solution before we ever get a failure!

so my take on this is that building using WITH_GMS=false is the way to go. only users of dyn-system devices will see the benefit of the change, but as time goes by more and more devices will fall in this class... so we better fix it.

@Lanchon
Copy link

Lanchon commented Aug 4, 2023

i want to add that today i was in the process of learning docker and building lineage images with a modded dockerfile. but suddenly everything stopped working and i can't even do a clean lineage build from the clean docker-lineage-cicd anymore, i don't know why. but builds are so slow and it's taking me a lot of time to debug. but the message is i'm working on it, and will hopefully make some progress soon.

i'm testing on instantnoodlep (oneplus 8 pro), in which system_a/b are dynamic partitions contained in super. flashing a standard los4mg image produces this:

OnePlus8Pro:/ # mount | grep '/ '                                                                                                                                                                                                            
/dev/block/dm-7 on / type ext4 (ro,seclabel,relatime,discard)
OnePlus8Pro:/ # df -h /                                                                                                                                                                                                                      
Filesystem      Size Used Avail Use% Mounted on
/dev/block/dm-7 824M 821M  2.5M 100% /system/bin/ziptool
OnePlus8Pro:/ # 

there's not much you can do with 2.5 MB these days... (this device has 256 GB storage.)

if you have a bunch of files and want to create a tight fitting filesystem for them, your job is not easy: filesystems are not designed to be used that way. android build possibly just adds up the sizes of files, adds a margin to that, and hopes it is good enough; which might work because the android OS does not include a lot of small files (it look like the margin is 2.5 MB). or it could round up each file size to block size (which?). or it could add a per-file space cost. or... who knows!

@Lanchon
Copy link

Lanchon commented Aug 4, 2023

my proposal to go forward...

given that lineageos already adds space to devices in which system is a dynamic partition, my proposal to go forward is to try reusing that code:

  1. apply this commit: Lanchon@d436725 (included in: Add WITH_MICROG setting  #460)
  2. build for a single dyn-system device and test. (i'm trying to do just that, but i'm not able to build anything right now, though i was successful yesterday... wth! i'm using instantnoodlep.)
  3. build for the whole roster and fix what doesn't build, if anything. (i won't be able to do the batch build, but i could debug particular devices that have issues.)
  4. if possible, test some more on real devices with those builds.
  5. publish and hope for few issue in the wild.

i'll try to start with step 1.

@Lanchon
Copy link

Lanchon commented Aug 4, 2023

currently stuck here: #468

@petefoth
Copy link
Contributor

petefoth commented Aug 5, 2023

Here's my final opinion on this issue.

  1. The WITH_GMS flag is 'owned' by LOS upstream, where it is used
    1. to enable the makefiles of the components in the vendor/partner/gms directory
    2. on some devices,
      • to free up some of the space that, in builds without the flag, is reserved for installing a GApps package
      • to make other device-specific changes to the amount of space reserved for system, product and other partitions.
  2. Our use of the the flag in our build commands is entirely consistent with 1: we set it true to enable the makefiles of the components in our vendor_partner_gms repo. That's all we do. Using the flag does not grab any space except the space taken up in the system and/or product images by the components that make our ROMs 'LineageOS for microG'. Given the name of the flag, I would argue that we are using the flag exactly as intended, to make a LineageOS ROM WITH_(MicroG's)GMS
  3. A problem has been reported, on one device, that one addon could not be installed because of lack of space in the system partition. That addon has been reported in the XDA forum as working fine on a different device. That does not appear to me to be a problem with our ROM. The root of the problem, and the place it should be fixed, is in the upstream device-specific makefiles for the problematic device. This isolated problem is certainly not a reason for us to change how we make our builds.
  4. If, in the future, upstream changes how they use the flag - or makes any other changes - in a way that affects how we make our builds, we will make any changes necessary in our code to allow us to continue building successfully. Until then, I see no need for us to change.

So, in my opinion, this issue is not highlighting any defect in our project, and there is therefore nothing that needs to be fixed. Much of the discussion of this issue concerns whether or not we should set `` WITH_GMS=true`. The fact is that we do set it, it works and, so far as I can tell it will continue to work .

So I intend to do the following:

  • Mark this defect with Wont fix and Not a bug labels, so that other users of this issue tracker can see what will be happening with the issue in the immediate future;
  • Mark similarly any other issues and PRs raised to "fix" this issue;
  • Leave these issues and PRs open, so that :
    • anyone interested can continue the discussion about what should or should not be done, , and whether or not it might work;
    • any other project maintainers can revisit and change my decision if they see fit to do so;
  • Ask @bananer - the other active maintainer of the project, to review this issue and my decisions / actions
  • Unsubscribe from the thread, so I can focus my efforts on some of the other issues and problems that are currently impeding our project goals of (continuing to) build and publish regular, up-to-date los4microg ROMs for around 200 devices.

I am sure many of the contributors to this thread will disagree with my opinion, my arguments and my actions. That is unfortunate, but I am acting in what I believe to be in the best interests of the project.

Thank you for all your contributions.

@infinity0
Copy link
Author

A problem has been reported, on one device, that one addon could not be installed because of lack of space in the system partition. [..]

From this one data point we can conclude by looking at the source code that any addon that needs to write extra stuff to the system partitions will break. Currently the only workaround is to convert the addon into a Magisk module instead, which does the same thing but dynamically at runtime.

The amount of comments on this issue indicate it's not just "one addon", I'm not sure where you are getting this "one addon" from. Just because it's not some popular addon doesn't mean it's not a bug, it's very very very difficult to debug and wasted several days of my time, multiply that by the number of people complaining here and you have some idea of how time-wasting it is. It discourages people from doing their own hobbyist Android development.

I'm not sure if it's obvious from the bits I quoted in the OP, but from the wider conversation with them (i.e. the actual LineageOS devs) they specifically told me you guys should not be setting WITH_GMS since it is an internal flag. Literally the exact opposite of "I would argue that we are using the flag exactly as intended".

If you don't believe me, go and talk to them on IRC, they are very reachable. (But don't say the word "microg" because they have/had a bot in there that auto-kicks anyone that says this, apparently they have a dim view of this project. Maybe try the -dev channel instead if you need to say the word.)

I'm honestly surprised at the amount of mental gymnastics some open source maintainers go through to convince themselves stuff is "not a bug". Lots of users are complaining, it's a bug by definition.

I also don't understand why there has been so much discussion here. It should be simple as setting a bunch of makefile flags to reserve some extra space, surely?

@petefoth
Copy link
Contributor

I'm closing this one now, along with the PR #460

@rozhuk-im
Copy link

rozhuk-im commented Jan 18, 2024

3. A problem has been reported, on one device, that one addon could not be installed because of lack of space in the system partition. That addon has been reported in the XDA forum as working fine on a different device. That does not appear to me to be a problem with our ROM. The root of the problem, and the place it should be fixed, is in the upstream device-specific makefiles for the problematic device. This isolated problem is certainly not a reason for us to change how we make our builds.

This is not true.

  1. One report only because LineageOS+MicroG is not friendly to get bug reports and I spend some time to figure out where is bugtracker.

  2. There is a lot of devices affected: https://github.com/search?q=org%3ALineageOS+WITH_GMS&type=code
    Only some samsung and may be some other devices not affected: https://github.com/LineageOS/android_device_samsung_sm7125-common/blob/65761014adf3ad1eeed134f1d0fd504557ee9f18/BoardConfigCommon.mk#L113

Setting WITH_GMS make no free space and no free inodes:
https://gitlab.com/LineageOS/issues/android/-/issues/6496

  1. This breaks many things that write/mod files in system partition.
    All things that use addon.d is broken, for me this is magisk auto patch on f/w update.
    Also I can not install Aurora services installer and many other things that I keep in system partition.

So, in my opinion, this issue is not highlighting any defect in our project, and there is therefore nothing that needs to be fixed. Much of the discussion of this issue concerns whether or not we should set `` WITH_GMS=true`. The fact is that we do set it, it works and, so far as I can tell it will continue to work .

All say: "this is not my bug, report in another place, closing".
I can not report about it in LOS - they will close bug because LOS builds without WITH_GMS and there is free space/inodes in system partitions.

@rozhuk-im
Copy link

As workaround/quick fix it is possible to patch on fly all BoardConfigCommon.mk files using find+sed to replace WITH_GMS with something that does not defined.

@petefoth
Copy link
Contributor

OK now we have one more bug report than we had before. That's not enough for us to change the way we make our builds.

My reasoning is that, in affected devices

  1. Without WITH_GMS set, LineageOS code leaves enough space in system partition for users to install GApps.
  2. With WITH_GMS set, that space does not need to be kept free because GApps will not / cannot be installed.

You may disagree with that reasoning, but I don't see any prospect of it changing!

As workaround/quick fix it is possible to patch on fly all BoardConfigCommon.mk files using find+sed to replace WITH_GMS with something that does not defined.

We build for ~230 devices, we are not going to patch all those BoardConfigCommon.mk files, on the fly or otherwise.

A number of possible fixes have been suggested (but not implemented or tested) in the issues and PRs mentioned above:if you want to try them out and make your own build that is up to you, but the project will not be making any such changes.

And I have spent wasted more than enough time on this non-issue, so I won't be responding further. If there is much more noise about his then I will lock the issue :)

@rozhuk-im
Copy link

I do not ask to change way how you make builds, WITH_GMS is your internal, I do not care set it or not, my issue is: no free space/inodes.
Just fix it in any way you like.

Free space on system partition required to many things, not only GApps.

If there is much more noise about his then I will lock the issue :)

Better remove bug tracker at all: no bug tracker = no bugs :)

@Lanchon
Copy link

Lanchon commented Jan 18, 2024

@petefoth sorry but your answer is bordering on ridiculous at this point. i don't care about you ~230 devices. i pleaded that at least you allowed a hidden, unsupported, undocumented flag to allow self-builders to build images by themselves without unnecessarily forking the project (#493) and you still didn't allow it; even thought it wouldn't affect a single of your 230 devices; even though this bug affects ALL android 10+ devices that use dynamic partitions (yes, ANY current device), as explained in the linked PR.

so my question at this point is:

who besides you have commit rights?

thank you for your help.

@petefoth
Copy link
Contributor

who besides you have commit rights?

All 'Members' of the lineageos4microg github organisation have commit rights. The current members of the organisation, and their organisation roles can be found here.

The 'Owners' of the organisation (of which I am not one), have rights to add new people to the organisation.

If you disagree with the project's decision on this issue and the related PRs, or with the way this issue and the associated PRs have been handled, then I guess you have the following options:

  • Contact the owners of the organisation and ask to be added as a member with commit rights;
  • Fork the project (as @Lanchon said he would do in this comment to #493). This is quite normal for open source projects, and is easy to do. If you need a Docker image to make builds without this flag, then you can fork the project, make the changes you require, and build a Docker image which incorporates your changes. You can push it to DockerHub for others to use if you wish. All of this is trivially easy to do: what you need to know is only an internet search away. I have done it for my personal projects, where I wish to do things differently from this project;
  • You can raise more issues and PRs here, but the organisation's decision on this issue is unlikely to change, and there is a good chance that any such issues will be closed without discussion.

I have spent wasted more than enough time on this non-issue, so I won't be responding further. If there is much more noise about his then I will lock the issue :)

There is no new information in recent posts, only people - including me - restating their opinions: to me this counts as 'noise'. I have therefore locked this issue and #493.

@lineageos4microg lineageos4microg locked and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants