Bug 1483129 - Review Request: golang-github-neurosnap-sentences - Multilingual command line sentence tokenizer in Golang
Summary: Review Request: golang-github-neurosnap-sentences - Multilingual command line...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1480762
TreeView+ depends on / blocked
 
Reported: 2017-08-18 19:44 UTC by Athos Ribeiro
Modified: 2017-09-11 19:10 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-09-11 19:10:39 UTC
Type: ---
Embargoed:
eclipseo: fedora-review+


Attachments (Terms of Use)

Description Athos Ribeiro 2017-08-18 19:44:02 UTC
Spec URL: https://athoscr.fedorapeople.org/packaging/golang-github-neurosnap-sentences.spec
SRPM URL: https://athoscr.fedorapeople.org/packaging/golang-github-neurosnap-sentences-1.0.6-1.fc26.src.rpm
Description: Multilingual command line sentence tokenizer in Golang
Fedora Account System Username: athoscr

Comment 1 Robert-André Mauchin 🐧 2017-08-19 08:21:56 UTC
%global import_path     %{provider_prefix}

You should use this as import path, but use the upstream provided one, gopkg.in/neurosnap/sentences.v1

%global import_path     gopkg.in/neurosnap/sentences.v1


And in the bottom of the spec:

%dir %{gopath}/src/gopkg.in/neurosnap/sentences.v1


Otherwise projects depending on this won't find it. Likewise you shouldn't patch the source in bug #1480762, it will find it once this is fixed.

Comment 2 Robert-André Mauchin 🐧 2017-08-19 13:47:59 UTC
s/should/shouldn't/

Comment 3 Athos Ribeiro 2017-08-19 16:30:43 UTC
What if some project decides to

import "github.com/neurosnap/sentences" ?

Note that upstream does so, here [1] and here [2]. So I see 3 possible options:

a) packaging this under both namespaces
b) package only one of the namespaces and patch the source code and dependencies to stick to only one namespace

(a) seems to be a good option at first, but then if we pay closer attention to the namespace in 'gopkg.in/neurosnap/sentences.v1', it carries the package major version with it. So whenever they decide to bump the major version, we would need to package it again and retire the old namespace. I believe this is not a good approach for software distribution. That's why I opted to go with (b).

If you prefer, we could go with (a), but packaging only gopkg.in/neurosnap/sentences.v1 is not possible because upstream itself would not build without the github package. A third option (c) would be to send a patch upstream to make [1] and [2] import gopkg.in/neurosnap/sentences.v1

But then this would still be a problem if some other project we want to package ever decides to import the github path for some reason (I do not have a solution for this).

[1] https://github.com/neurosnap/sentences/blob/eaa759e51378560dd9b545de7502d7c92c5166a7/_cmd/sentences/main.go#L10

[2] https://github.com/neurosnap/sentences/blob/c4a4a02cd25ad0d1b1b623459a63fbeceae6c375/sentences_test.go#L8

Comment 4 Robert-André Mauchin 🐧 2017-08-19 16:40:12 UTC
You can go with (a) and define a x_provider, x_provider_tld, x_project, x_repo and x_import_path for the second one, then add the list of provides with the second import_path, then do a symlink between the two:

ln -sT %{gopath}/src/%{import_path} %{buildroot}%{gopath}/src/%{x_import_path}

Comment 5 Athos Ribeiro 2017-08-19 17:23:13 UTC
You mean, packaging 2 different -devel subpackages, like the extra packages from golang.org/x/foo? That works for me. I will prepare sth and update the sources as soon as I can.

Comment 6 Robert-André Mauchin 🐧 2017-08-19 17:53:38 UTC
No. No need for two -devel packages, just one with the double provides and a symlink. I have a sample somewhere, see this spec: https://github.com/eclipseo/packaging/blob/master/golang-googlecode-image.spec

Comment 7 Athos Ribeiro 2017-08-21 16:59:29 UTC
Makes sense!

I will apply the changes as soon as I can. Thanks for the input and example

Comment 8 Athos Ribeiro 2017-09-08 21:13:00 UTC
Hi Robert,

Here is the updated package. I have one point worth mentioning here:

- I am not sure what to do with the '/usr/share/gocode/src/gopkg.in' directry. Usually, the golang package owns the directories in that level, as it does with the /usr/share/gocode/src/github.com one. I will ping the golang package maintainers about it to check if they are willing to include such directory in the package. Currently, I am not owning it in this package.


Spec URL: https://athoscr.fedorapeople.org/packaging/golang-github-neurosnap-sentences.spec
SRPM URL: https://athoscr.fedorapeople.org/packaging/golang-github-neurosnap-sentences-1.0.6-2.fc26.src.rpm

Comment 9 Robert-André Mauchin 🐧 2017-09-10 13:53:17 UTC
Everything seems in order, package accepted.

Comment 10 Gwyn Ciesla 2017-09-11 14:16:58 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/golang-github-neurosnap-sentences


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