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

Remove all modularity support #4945

Merged
merged 1 commit into from
May 23, 2024

Conversation

cgwalters
Copy link
Member

Derived from #4937

Part of https://fedoraproject.org/wiki/Changes/RetireModularity

It's dead, so let's remove all the code. I think this may
cause things to silently do nothing if folks have modules
layered, but at least server side builds will just stop working.

Copy link

openshift-ci bot commented May 2, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2174317"
kver=6.2.8
koji_kernel_url=https://koji.fedoraproject.org/koji/buildinfo?buildID=2294111
kver=6.5.5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
kver=6.5.5
kver=6.8.5

url_suffix=2.16.2/2.fc39/x86_64/ignition-2.16.2-2.fc39.x86_64.rpm
# 2.15.0-3
koji_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2158585"
koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2174317"
kver=6.2.8
koji_kernel_url=https://koji.fedoraproject.org/koji/buildinfo?buildID=2294111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
koji_kernel_url=https://koji.fedoraproject.org/koji/buildinfo?buildID=2294111
koji_kernel_url=https://koji.fedoraproject.org/koji/buildinfo?buildID=2435097

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are testing for the version ID, so either we change that or we use a package from the same version(f40)

Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks we need to remove:

        let modules = extensions.modules.unwrap();
        assert!(modules.enable.unwrap().contains("virt:av"));
        assert!(modules.install.is_none());

and

        let modules = extensions.modules.unwrap();
        assert!(modules.enable.as_ref().unwrap().contains("foo:stable"));
        assert!(modules.enable.as_ref().unwrap().contains("virt:av"));
        assert!(modules.install.is_none())

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, happy to see it go. On the origin side, possibly we need code to actually drop the dead keys so they don't just hang around (I mean, it's harmless but it's also misleading).

@jlebon
Copy link
Member

jlebon commented May 3, 2024

This is continued in #4937 again.

@jlebon jlebon closed this May 3, 2024
@cgwalters
Copy link
Member Author

They're different, the change in 1ba3a7b just deletes the entrypoints to minimize code churn. This PR deletes it all.

@HuijingHei
Copy link
Member

From failed logs, might need to remove related in src/libpriv/rpmostree-util.cxx

May 22 02:58:25.162432 kola-runext.service[2197]: + rpm-ostree override remove chrony
...
May 22 02:58:50.402554 kola-runext.service[2243]: Writing OSTree commit...done
May 22 02:58:54.682538 rpm-ostreed.service[1645]: **
May 22 02:58:54.682538 rpm-ostreed.service[1645]: rpm-ostreed:ERROR:src/libpriv/rpmostree-util.cxx:510:gboolean rpmostree_deployment_get_layered_info(OstreeRepo*, OstreeDeployment*, gboolean*, guint*, char**, char***, char***, GVariant**, GVariant**, GVariant**, GError**): assertion failed: (g_variant_dict_lookup (dict, "rpmostree.modules", "^as", &layered_modules))
May 22 02:58:54.684000 audit[1645]: ANOM_ABEND auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:install_t:s0 pid=1645 comm="tokio-runtime-w" exe="/usr/bin/rpm-ostree" sig=6 res=1
May 22 02:58:54.684891 rpm-ostreed.service[1645]: Bail out! rpm-ostreed:ERROR:src/libpriv/rpmostree-util.cxx:510:gboolean rpmostree_deployment_get_layered_info(OstreeRepo*, OstreeDeployment*, gboolean*, guint*, char**, char***, char***, GVariant**, GVariant**, GVariant**, GError**): assertion failed: (g_variant_dict_lookup (dict, "rpmostree.modules", "^as", &layered_modules))

Part of https://fedoraproject.org/wiki/Changes/RetireModularity

It's dead, so let's remove all the code.  I think this may
cause things to silently do nothing if folks have modules
layered, but at least server side builds will just stop working.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Member Author

OK, if CI passes on this one I think we can call it good to go.

@cgwalters cgwalters merged commit ac47bb4 into coreos:main May 23, 2024
17 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.

5 participants