Bug 2373038 - Review Request: elementary-photos - Photo manager and viewer from elementary
Summary: Review Request: elementary-photos - Photo manager and viewer from elementary
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Steve Cossette
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/elementary/photos
Whiteboard:
Depends On:
Blocks: PantheonDesktopPackageReviews
TreeView+ depends on / blocked
 
Reported: 2025-06-16 17:12 UTC by Fabio Valentini
Modified: 2025-07-07 12:28 UTC (History)
2 users (show)

Fixed In Version: elementary-photos-8.0.1-1.fc43
Clone Of:
Environment:
Last Closed: 2025-07-07 12:28:17 UTC
Type: ---
Embargoed:
farchord: fedora-review+


Attachments (Terms of Use)

Description Fabio Valentini 2025-06-16 17:12:54 UTC
Spec URL: https://decathorpe.fedorapeople.org/elementary-photos.spec
SRPM URL: https://decathorpe.fedorapeople.org/elementary-photos-8.0.1-1.fc42.src.rpm

Description:
The elementary continuation of Shotwell, originally written by Yorba
Foundation.

Fedora Account System Username: decathorpe

Comment 1 Fabio Valentini 2025-06-16 17:12:57 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=134046251

Comment 2 Fedora Review Service 2025-06-17 05:18:30 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9168867
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2373038-elementary-photos/fedora-rawhide-x86_64/09168867-elementary-photos/fedora-review/review.txt

Found issues:

- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/elementary-photos
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names

Please know that there can be false-positives.

---
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.

Comment 3 Steve Cossette 2025-06-20 01:32:32 UTC
A couple preliminary comments:

1- You might want to implement fdupes in your spec. Adding it as a BR and doing %fdupes %{buildroot} will find all the duplicate files and symlink them, lowering the file size.
2- This should probably be looked into: E: incorrect-fsf-address /usr/share/licenses/elementary-photos/COPYING (At least a PR might be a good idea)
3- The following should probably be added as Requires:

Run-time dependency glib-2.0 found: YES 2.85.1
Run-time dependency gio-unix-2.0 found: YES 2.85.1
Run-time dependency gee-0.8 found: YES 0.20.8
Run-time dependency gexiv2 found: YES 0.14.5
Run-time dependency geocode-glib-2.0 found: YES 3.26.4
Run-time dependency gmodule-2.0 found: YES 2.85.1
Run-time dependency gstreamer-1.0 found: YES 1.26.2
Run-time dependency gstreamer-base-1.0 found: YES 1.26.2
Run-time dependency gstreamer-plugins-base-1.0 found: YES 1.26.2
Run-time dependency gstreamer-pbutils-1.0 found: YES 1.26.2
Run-time dependency granite found: YES 6.2.0
Run-time dependency gtk+-3.0 found: YES 3.24.49
Run-time dependency gudev-1.0 found: YES 238
Run-time dependency libhandy-1 found: YES 1.8.3
Run-time dependency libexif found: YES 0.6.25
Run-time dependency libgphoto2 found: YES 2.5.31
Run-time dependency libraw found: YES 0.21.4
Run-time dependency libwebp found: YES 1.5.0
Run-time dependency libportal found: YES 0.9.1
Run-time dependency libportal-gtk3 found: YES 0.9.1
Run-time dependency sqlite3 found: YES 3.50.0

(They are added as a build requirement, but as they are runtime, they will be needed on runtime)

Once that's done I can look into doing the review!

Comment 4 Fabio Valentini 2025-06-20 09:29:38 UTC
Thank you for your comments!

> 1- You might want to implement fdupes in your spec. Adding it as a BR and doing %fdupes %{buildroot} will find all the duplicate files and symlink them, lowering the file size.

Why? I'm not aware of any large number of "duplicate" files in this package.
Additionally, the current implementation of %fdupes is brittle and causes non-reproducible package builds, so I would rather avoid it unless it really provides big benefits.

> 2- This should probably be looked into: E: incorrect-fsf-address /usr/share/licenses/elementary-photos/COPYING (At least a PR might be a good idea)

Thanks, I'll report this upstream. It shouldn't be a blocker though.

> 3- The following should probably be added as Requires:

As far as I can tell, this is just a weird way that meson prints output when looking for dependencies.

The executable in this package already dynamically links to all these libraries, so RPM already generates explicit Requires for all of them (as you can see from the build.log or the Requires in the fedora-review.txt).

Comment 5 Steve Cossette 2025-06-20 10:02:38 UTC
(In reply to Fabio Valentini from comment #4)
> Thank you for your comments!
> 
> > 1- You might want to implement fdupes in your spec. Adding it as a BR and doing %fdupes %{buildroot} will find all the duplicate files and symlink them, lowering the file size.
> 
> Why? I'm not aware of any large number of "duplicate" files in this package.
> Additionally, the current implementation of %fdupes is brittle and causes
> non-reproducible package builds, so I would rather avoid it unless it really
> provides big benefits.
> 

Yeah it's not a huge benefit, it's about a quarter of a megabyte. But there was quite a few duplicates. I can forget about that though.

> > 2- This should probably be looked into: E: incorrect-fsf-address /usr/share/licenses/elementary-photos/COPYING (At least a PR might be a good idea)
> 
> Thanks, I'll report this upstream. It shouldn't be a blocker though.

Right, it isn't.
> 
> > 3- The following should probably be added as Requires:
> 
> As far as I can tell, this is just a weird way that meson prints output when
> looking for dependencies.
> 
> The executable in this package already dynamically links to all these
> libraries, so RPM already generates explicit Requires for all of them (as
> you can see from the build.log or the Requires in the fedora-review.txt).

Yeah I guess you're right, I just missed it.

With the issue with the license reported upstream, a local build/install worked, and everything looks good to me.

So this review is approved!

Comment 6 Fabio Valentini 2025-07-07 12:28:17 UTC
Imported and built:
https://bodhi.fedoraproject.org/updates/FEDORA-2025-1466e8f374


Note You need to log in before you can comment on or make changes to this bug.