Bug 1369560

Summary:	Review Request: snapd-xdg-open - Bridge allowing (untrusted) snap applications to use xdg-open
Product:	[Fedora] Fedora	Reporter:	Zygmunt Krynicki <me>
Component:	Package Review	Assignee:	Nobody's working on this, feel free to take it <nobody>
Status:	CLOSED WONTFIX	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	morphis, ngompa13, package-review
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2017-09-01 17:53:47 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Bug Depends On:
Bug Blocks:	201449

Description Zygmunt Krynicki 2016-08-23 18:44:45 UTC

Spec URL: https://fedorapeople.org/~zyga/snapd-xdg-open.spec
SRPM URL: https://fedorapeople.org/~zyga/snapd-xdg-open-0.0.0-1.fc24.src.rpm
Description: 
A D-Bus-activated helper service allowing snaps to launch URLs on the host
where its installed. The service validates and checks the requested URLs before
allowing them to be opened.

Fedora Account System Username: zyga

Comment 1 Neal Gompa 2016-08-23 18:45:45 UTC

Taking this review.

Comment 2 Neal Gompa 2016-08-25 04:19:23 UTC

Initial issues:

* Not properly set up for pre-release. It is perfectly acceptable to package up snapshot pre-releases, provided that the guidelines are followed for versioning them: https://fedoraproject.org/wiki/Packaging:Versioning#Pre-Release_packages

* URL is invalid. While it does properly redirect, I would strongly prefer to see the correct URL set here, which I understand it to be "https://github.com/snapcore/snapd-xdg-open"

* Source0 is invalid. This is a serious problem. Now, since you're currently using snapshots, you should follow the guidelines for snapshot URLs, as mentioned here: https://fedoraproject.org/wiki/Packaging:SourceURL#Commit_Revision

A suggestion: You can set the source URL to "https://github.com/snapcore/snapd-xdg-open/archive/%{commit0}/%{name}-%{shortcommit0}.tar.gz", which will work with spectool to produce a proper snapshot. You'll need to add "-n %{name}-%{commit0}" to the %setup invocation, also.

Comment 3 Neal Gompa 2017-03-31 15:07:18 UTC

No response in months, marking as DEADREVIEW.

Comment 4 Simon Fels 2017-03-31 17:08:05 UTC

I've updated the spec file and produced a new SRPM with all necessary changes to fit Neals review comments.

You can find the new files at

 * https://mm.gravedo.de/files/snapd-xdg-open.spec
 * https://mm.gravedo.de/files/snapd-xdg-open-0.0.0-0.1.git6fed357.fc25.src.rpm

Comment 5 Neal Gompa 2017-03-31 19:09:53 UTC

@Simon:

Please open a new review request and close this one as a duplicate of it.

Comment 6 Neal Gompa 2017-09-01 17:53:47 UTC

This has been obsoleted by the re-implementation of this code in snapd[1].

[1]: https://github.com/snapcore/snapd/commit/047538e13d0178e30d7cba551ac5cf41c57b78b5