Bug 2368737
| Summary: | Review Request: cef - Chromium Embedded Framework | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Asahi Lina <lina> |
| Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | dominik, ngompa13, package-review |
| Target Milestone: | --- | Flags: | ngompa13:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | --- | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2025-06-18 08:17:01 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Asahi Lina
2025-05-27 12:31:33 UTC
Note: OBS is using CEF 133 right now. I'm hoping it can use the versioned API, or that it will build fine with 136 anyway if it requires the EXPERIMENTAL API. If it doesn't I plan to just patch it to make it work instead of rolling back the CEF version. Taking this review. Copr build: https://copr.fedorainfracloud.org/coprs/build/9092281 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368737-cef/fedora-rawhide-x86_64/09092281-cef/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string. This package is essentially forked from Chromium with the CEF bits merged in.
Review notes:
* Package generally follows Fedora Packaging Guidelines
* Licensing is the same as Chromium so looks good there
* Package builds and installs locally (verified aarch64)
* Generated mutually exclusive subpackages have correct dependencies, but the RemovePathPostfixes doesn't seem to have worked.
- Examining the generated spec makes me think it's not used correctly
Are you sure "${postfix:+RemovePathPostfixes: }${postfix}" is correct?
For "cef-api13301-devel", I would expect "RemovePathPostfixes: .api13301/" as the string to "strip".
It worked for the files but not the directory, so I think it's just the trailing slash issue we discussed. I'll just remove the trailing slash in the file list. Unfortunately it looks like RemovePathPostfixes: does not work for directories. It causes the directory itself to have the postfix removed in its own entry, but not the entries for the files within.
RemovePathPostfixes: .suffix
%install
mkdir -p %{buildroot}/dir.suffix
touch %{buildroot}/foo.suffix
touch %{buildroot}/dir.suffix/bar
%files
/foo.suffix
/dir.suffix
Results in these RPM contents:
/dir
/dir.suffix/bar
/foo
Playing around with the trailing slash doesn't seem to change anything. I think I can work around this, though.
Can you please file a bug with RPM about this too? I think the fact it doesn't work for directories is kind of unexpected. https://github.com/rpm-software-management/rpm/issues Filed: https://github.com/rpm-software-management/rpm/issues/3775 I have a workaround using file lists, running a build now to see if it works. Updated the spec file and mkspec.sh. This is now verified with a (manual) OBS compile against a versioned API, testing an end-to-end rebuild of CEF now. The biggest issue was that the CEF wrapper was getting built with clang, and CEF headers introduce a clang-only C++ ABI break deliberately. I patched that out, so hopefully the devel wrapper libs are compatible with both compilers now. Either way, I also switched the wrapper build to g++ just in case. At this point, things look good to me. PACKAGE APPROVED. The Pagure repository was created at https://src.fedoraproject.org/rpms/cef After speaking to the CEF folks and thinking about this some more, we decided to drop the idea of packaging a prebuilt libcef_wrapper and instead ship the source code plus a FindCEF.cmake module that sets up the library target to build as part of the consumer. This more closely follows how CEF is intended to be used, and means consumers can configure their own build flags for the CEF wrapper lib. It also simplifies the spec file and removes some footguns. So now there is a single `cef-devel` package that ships the wrapper source, which may be built for any given library API version. This also simplifies the OBS package changes. Now the only change needed is to just remove the FindCEF.cmake that OBS ships, and then it will use the system one and everything works. The CEF API version can be specified via CMake define. For reference, these are the WIP changes to the spec file: https://gist.github.com/asahilina/a64976b6bad98e7df9d42900f963cf6f And this is FindCEF.cmake: https://gist.github.com/asahilina/bc3537becfb397b9303278fc09dc06ef FEDORA-2025-3bbac57425 (cef-137.0.17^chromium137.0.7151.103-1.fc43) has been submitted as an update to Fedora 43. https://bodhi.fedoraproject.org/updates/FEDORA-2025-3bbac57425 FEDORA-2025-3bbac57425 (cef-137.0.17^chromium137.0.7151.103-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report. FEDORA-2025-c4abcadb56 (cef-137.0.17^chromium137.0.7151.103-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2025-c4abcadb56 FEDORA-2025-c4abcadb56 (cef-137.0.17^chromium137.0.7151.103-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report. |