Implement EULA spoke based on initial-setup #4687

mikhailnov · 2023-04-07T13:03:38Z

Copied code from initial-setup and adapted it for Anaconda.

It makes sense to show EULA before installation, not just text that it is in some file after the installation.

TODO: It would be great to automatically choose license in the current language. Currently it is worked around in ROSA by creating a drop-in config: https://abf.io/import/anaconda/blob/6dc5b78efd/anaconda.sh#lc-77

Closes: #4686

It looks like this:

Here is Russian translation of new strings (they were not inside po and pot files in initial-setup):

msgid "LICENSING"
msgstr "ЛИЦЕНЗИРОВАНИЕ"

msgctxt "GUI|Spoke"
msgid "_License Information"
msgstr "О _лицензии"

msgid "License information"
msgstr "О лицензии"

msgid "License Information"
msgstr "О лицензии"

msgid "License accepted"
msgstr "Лицензия принята"

msgid "License not accepted"
msgstr "Лицензия не принята"

msgid "Read the License Agreement"
msgstr "Прочитайте лицензионное соглашение"

msgid "I accept the license agreement"
msgstr "Принимаю лицензионное соглашение"

msgid "I _accept the license agreement"
msgstr "_Принимаю лицензионное соглашение"

msgid "License Agreement:"
msgstr "Лицензионное соглашение:"

In some places translation is not exact because otherwise words will be too long.

ISO image (LiveCD + installer) with Anaconda with EULA spoke: https://file-store.rosalinux.ru/download/2aaf53506bc7c689fa81ab1de078619afa6db568 (ROSA_2021.1_XFCE_x86_64_8.iso)

VladimirSlavik

Thanks! This looks good.

A couple of thoughts, in no particular order:

Please add license to the code files, and don't forget to set correct year :)
Probably needs team discussion if this should be a community feature or not. Personally I think that's fine as-is, there is very little code and it's usable primarily for RHEL anyway.
It's debatable if the spoke should be also disabled by the default config or not. Ultimately that's a minor point, though.
Note to self - the agreed/disagreed state needs to be stored in modularized backend, eventually.

VladimirSlavik · 2023-04-11T15:26:30Z

pyanaconda/ui/gui/spokes/eula.py

+__all__ = ["EULASpoke"]
+
+
+class EULASpoke(FirstbootOnlySpokeMixIn, NormalSpoke):


FirstbootOnlySpokeMixIn practically says it runs only in initial setup. Is that really the right parent to use? I'd expect only NormalSpoke here...

I expected this spoke to be reusable by initial-setup and remove it from there after merging here in Anaconda. But I have not tested it properly with initial-setup yet.

Initial setup has not been touched for a long time, is not enough of a priority to get actual dev time, and we're asking around to see if we can just drop it completely. So I'd expect it to just keep its own eula spoke and this be in anaconda only.

Yeah, that might make more sense - not that much code is needed for EULA display, so it might make sense to just duplicate it in Anaconda, rather than to keep the screen reusable, with that special mixin and the necessary Initial Setup changes.

VladimirSlavik · 2023-04-11T15:47:40Z

pyanaconda/ui/gui/spokes/eula.py

+    @classmethod
+    def should_run(cls, environment, data):
+        if eula.eula_available():
+            # don't run if we are in initial-setup in reconfig mode and the EULA has already been accepted
+            if FirstbootOnlySpokeMixIn.should_run(environment, data) and data and data.firstboot.firstboot == FIRSTBOOT_RECONFIG and data.eula.agreed:
+                log.debug("not running license spoke: reconfig mode & license already accepted")
+                return False
+            return True
+        return False


This needs changing a bit. All anaconda spokes work such that they are always visible, and kickstart changes only if they are completed or not.

Since the spoke is not reused by initial setup, I think you can leave out the FIRSTBOOT_RECONFIG part. @M4rtinK - is that correct please?

The spoke visibility in should_run should depend only on eula presence.

The data.eula.agreed bit should set up the checkbox, I think in initialize.

The data.eula.agreed bit should set up the checkbox, I think in initialize

Do you mean running GUI/TUI with a kickstart with eula --agreed? What is the use case for this, who and why will do that? Is it possible without hackery?

Yes, it's actually very easy. If you don't make anaconda stop on incomplete kickstart, you can use a "partial kickstart" that does not satisfy all needed inputs, and the UIs will be pre-filled when it stops on the hub. It's a thing that is not really nice to support, but invaluable for testing. I run 99% of my anaconda debugging with this, so that I don't have to set it all every time I retry my changes :P Some spins also use this, except hidden in the "default kickstart", together with hiding spokes in config, to set up things that users can't change. It's a bit of a dark art, really.

The rest of the UI works this way, so it would be best to make it consistent.

Since the spoke is not reused by initial setup, I think you can leave out the FIRSTBOOT_RECONFIG part. @M4rtinK - is that correct please?

Yes, I think it really makes more sense to just add the ability to Anaconda to show the EULA without also trying to reuse that screen from Initial Setup. This nicely shows the benefits of that - we can drop the Initial setup specific mixin (which still refers to the old "Firstboot" tool, to be even more confusing) as well as the Initial Setup specific reconfig functionality.

As for when the EULA spoke should show up - I think we should just check if an EULA is in the expected place & show it if there is something. That's how it works for Initial Setup & should work for the installer as well.

pyanaconda/ui/gui/spokes/eula.py

VladimirSlavik · 2023-04-17T16:07:17Z

From team discussion today:

Sounds good to have it, unless there's substantial problems moving this to modularized backend.
Preferably land this before converting the remaining kickstart commands to use the modularized backend. That's planned for this quarter. The mechanism for handling the eula command in backend needs adding, but that is low overhead on top of handling the command at all.
Unknowns about the above are yet unknown...
Adding another GUI category sounds like a problem, categories are a nice idea but reality looks like overbooked "system" pushing other things below the screen end and adding scrollbars. Personally I think putting this under "Software" would be best, but that's something of a detail. Technical feasibility is the first hurdle.

VladimirSlavik · 2023-05-25T16:51:03Z

Note to self: The eula command would be in Payloads module, spoke under Software category. Right now, this is low priority compared to modularization for webui in f39 (hopefully).

mikhailnov · 2023-06-27T18:21:17Z

Sorry for very long delay :(

Me and we in ROSA like the initial-setup utility. We began using it recently and like it. It has a nice architecture and nice integration with Anaconda. Its modularity (ability to add spokes) makes it be a great thing, better than e.g. GNOME's initial setup. Its UI is probably not the nicest, but it's OK, it looks good enough, and custom CSS can be written in theory.

We are now making it possible to use initial-setup as the first time configuration wizard for OEM installations: a user buys a computer with Linux preinstalled and configures it via initial-setup (user, root password, EULA, custom spokes may be added).

We are going to use the EULA spoke in both Anaconda and initial-setup, that is why I would like to avoid removing possibility to reuse it by initial-setup. but of course I can make it as you suggest here in Anaconda upstream and patch it in the downstream, but it is not he best way...

Copied code from initial-setup and adapted it for Anaconda. It makes sense to show EULA before installation, not just text that it is in some file after the installation. TODO: It would be great to automatically choose license in the current language. Currently it is worked around in ROSA by creating a drop-in config: https://abf.io/import/anaconda/blob/6dc5b78efd/anaconda.sh#lc-77 Closes: rhinstaller#4686 Signed-off-by: Mikhail Novosyolov <m.novosyolov@rosalinux.ru>

mikhailnov · 2023-06-27T18:58:25Z

I've implemented reading "eula --agreed" from kickstart in GUI. It was implemented in TUI.

I can also remove integration with initial-setup, but I would better leave it here in Anaconda and remove EULA code from initial-setup.

mikhailnov · 2023-07-04T19:35:09Z

Could you please give comments, should I make further changes?

VladimirSlavik · 2023-08-16T13:06:24Z

Sorry, we were focused on the WebUI moonshot, which is sort of undecided until beta (freeze?), I believe. I'll take a look again once there's time to do so.

mikhailnov · 2023-08-16T13:59:53Z

Ok, thanks)

github-actions · 2023-10-16T02:05:48Z

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

Conan-Kudo · 2023-10-25T13:15:24Z

@VladimirSlavik ping

KKoukiou · 2023-11-14T09:47:35Z

Hi @mikhailnov, thank you for your contribution! After thorough discussion in our team meeting, it has been determined that presenting this feature as an add-on would be more in line with our current objectives. While the anaconda team acknowledges the importance of this work, it is our preference to encourage the development of this as an add-on. The responsibility for maintaining and supporting this feature as an add-on would primarily rest with the contributor, with the anaconda team available for guidance as needed.
We've identified a few considerations against the current approach:

The current PR doesn't build on a modularized API, which is a requirement for new features.
The EULA spoke may not be necessary for all supported distros (e.g., not needed for RHEL/Centos), as it's assumed that users confirm the license agreement when downloading the ISO.
Introducing new features in the GUI only, is not aligned with the team's strategy to move slowly towards the new Web UI solution.
We aim to maintain the existing installer behavior, allowing users to perform non-interactively installations, by configuring everything through kickstart. Introducing a new option as is in this PR might will the current behavior.

Given these factors, we suggest reconsidering this as an add-on.

Would you be open to taking this approach? The team is committed to actively supporting you during the migration process. Apologies for the delayed response, and we appreciate your understanding.

VladimirSlavik added the f39 label Apr 11, 2023

VladimirSlavik reviewed Apr 11, 2023

View reviewed changes

VladimirSlavik added the release note required Write a release note for this change. label Apr 11, 2023

VladimirSlavik self-assigned this Apr 11, 2023

VladimirSlavik requested a review from M4rtinK April 11, 2023 16:02

mikhailnov force-pushed the license-spoke branch from cf4a883 to e6f62b5 Compare June 27, 2023 18:56

github-actions bot added the stale label Oct 16, 2023

github-actions bot removed the stale label Oct 26, 2023

KKoukiou added blocked Don't merge this pull request! and removed release note required Write a release note for this change. f39 labels Nov 14, 2023

KKoukiou closed this Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement EULA spoke based on initial-setup #4687

Implement EULA spoke based on initial-setup #4687

mikhailnov commented Apr 7, 2023 •

edited

Loading

VladimirSlavik left a comment

VladimirSlavik Apr 11, 2023

mikhailnov Apr 11, 2023

VladimirSlavik Apr 11, 2023 •

edited

Loading

M4rtinK Apr 14, 2023

VladimirSlavik Apr 11, 2023

mikhailnov Apr 11, 2023

VladimirSlavik Apr 11, 2023

M4rtinK Apr 14, 2023

VladimirSlavik commented Apr 17, 2023 •

edited

Loading

VladimirSlavik commented May 25, 2023

mikhailnov commented Jun 27, 2023

mikhailnov commented Jun 27, 2023

mikhailnov commented Jul 4, 2023

VladimirSlavik commented Aug 16, 2023

mikhailnov commented Aug 16, 2023

github-actions bot commented Oct 16, 2023

Conan-Kudo commented Oct 25, 2023

KKoukiou commented Nov 14, 2023 •

edited

Loading

		__all__ = ["EULASpoke"]


		class EULASpoke(FirstbootOnlySpokeMixIn, NormalSpoke):

Implement EULA spoke based on initial-setup #4687

Implement EULA spoke based on initial-setup #4687

Conversation

mikhailnov commented Apr 7, 2023 • edited Loading

VladimirSlavik left a comment

Choose a reason for hiding this comment

VladimirSlavik Apr 11, 2023

Choose a reason for hiding this comment

mikhailnov Apr 11, 2023

Choose a reason for hiding this comment

VladimirSlavik Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

M4rtinK Apr 14, 2023

Choose a reason for hiding this comment

VladimirSlavik Apr 11, 2023

Choose a reason for hiding this comment

mikhailnov Apr 11, 2023

Choose a reason for hiding this comment

VladimirSlavik Apr 11, 2023

Choose a reason for hiding this comment

M4rtinK Apr 14, 2023

Choose a reason for hiding this comment

VladimirSlavik commented Apr 17, 2023 • edited Loading

VladimirSlavik commented May 25, 2023

mikhailnov commented Jun 27, 2023

mikhailnov commented Jun 27, 2023

mikhailnov commented Jul 4, 2023

VladimirSlavik commented Aug 16, 2023

mikhailnov commented Aug 16, 2023

github-actions bot commented Oct 16, 2023

Conan-Kudo commented Oct 25, 2023

KKoukiou commented Nov 14, 2023 • edited Loading

mikhailnov commented Apr 7, 2023 •

edited

Loading

VladimirSlavik Apr 11, 2023 •

edited

Loading

VladimirSlavik commented Apr 17, 2023 •

edited

Loading

KKoukiou commented Nov 14, 2023 •

edited

Loading