Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1709481 - Review Request: vim-ctrlp - Full path fuzzy file, buffer, mru, tag, ... finder for Vim
Summary: Review Request: vim-ctrlp - Full path fuzzy file, buffer, mru, tag, ... finde...
Keywords:
Status: CLOSED ERRATA
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: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2019-05-13 16:59 UTC by Ondrej Soukup
Modified: 2019-08-09 01:02 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-08-09 01:02:43 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Ondrej Soukup 2019-05-13 16:59:53 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/osoukup/ctrlp.vim/fedora-rawhide-x86_64/00906845-vim-ctrlp/vim-ctrlp.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/osoukup/ctrlp.vim/fedora-rawhide-x86_64/00906845-vim-ctrlp/vim-ctrlp-1.8.0-1.fc31.src.rpm
Description:

Hi anyone! I am enthusiastic VIM user and I really like vim.ctrlp plugin which adds awesome easy-to-use but powerful search capabilities. You can find upstream repo at

https://github.com/ctrlpvim/ctrlp.vim

After some time of using it I think that it might be nice to have it as official Fedora package the same way as some other great VIM plugins are. I prepared everything I found out is necessary and now would be very glad for some review. I am new to Fedora packaging so I appreciate any advice or help.

For the sake of completeness here are the links to COPR project and KOJI scratch build

https://copr.fedorainfracloud.org/coprs/osoukup/ctrlp.vim/
https://koji.fedoraproject.org/koji/taskinfo?taskID=34834617

For the testing purposes I have created personal public repo where anyone can find scripts to easily test the RPM build and install in Vagrant with libvirt VM

https://github.com/osoukup/ctrlp.vim

Fedora Account System Username: osoukup

Comment 1 Ondrej Soukup 2019-05-14 07:45:53 UTC
Adding also rpmlint reports:

---------------------

$ rpmlint -i SRPMS/vim-ctrlp-1.8.0-1.fc30.src.rpm 
vim-ctrlp.src: W: spelling-error Summary(en_US) mru -> mu, rum
The value of this tag appears to be misspelled. Please double-check.

vim-ctrlp.src: W: spelling-error %description -l en_US mru -> mu, rum
The value of this tag appears to be misspelled. Please double-check.

vim-ctrlp.src: W: spelling-error %description -l en_US gVim -> g Vim, grim, vim
The value of this tag appears to be misspelled. Please double-check.

vim-ctrlp.src: W: invalid-url Source0: vim-ctrlp-1.8.0.tar.gz
The value should be a valid, public HTTP, HTTPS, or FTP URL.

1 packages and 0 specfiles checked; 0 errors, 4 warnings.

---------------------

$ rpmlint -i RPMS/noarch/vim-ctrlp-1.8.0-1.fc30.noarch.rpm 
vim-ctrlp.noarch: W: spelling-error Summary(en_US) mru -> mu, rum
The value of this tag appears to be misspelled. Please double-check.

vim-ctrlp.noarch: W: spelling-error %description -l en_US mru -> mu, rum
The value of this tag appears to be misspelled. Please double-check.

vim-ctrlp.noarch: W: spelling-error %description -l en_US gVim -> g Vim, grim, vim
The value of this tag appears to be misspelled. Please double-check.

1 packages and 0 specfiles checked; 0 errors, 3 warnings.

---------------------

That mru spelling error in the description should IMO refer to most recently used which is correct abbreviation. The description was however taken unchanged from the upstream:

http://ctrlpvim.github.io/ctrlp.vim/

Comment 2 Robert-André Mauchin 🐧 2019-06-05 20:49:42 UTC
 - Source0 must be a url pointing to the archive

Source0:        https://github.com/ctrlpvim/ctrlp.vim/archive/%{version}/%{name}-%{version}.tar.gz

 - Version is 1.80, not 1.8.0

 - Appdata dir is the obsolete location, Appdata files now go in %{_metainfodir}

 - Add vim-filesystem as a RR too

Requires:       vim-filesystem

Comment 3 Ondrej Soukup 2019-06-10 11:01:20 UTC
Thank you very much for the review!

(In reply to Robert-André Mauchin from comment #2)
>  - Source0 must be a url pointing to the archive
> 
> Source0:       
> https://github.com/ctrlpvim/ctrlp.vim/archive/%{version}/%{name}-%{version}.
> tar.gz

I updated Source0 and also Source1 (hope I am not too eager) to URLs instead of file system paths.

>  - Version is 1.80, not 1.8.0

I corrected the version number. I have to admit that I was confused by the upstream since they use to use X.Y.Z versions but switched to X.YZ for some reason. You are right that I should respect their version notation. 

>  - Appdata dir is the obsolete location, Appdata files now go in
> %{_metainfodir}

Directory for appdata files was also corrected according to your suggestion.
 
>  - Add vim-filesystem as a RR too
> 
> Requires:       vim-filesystem

The requirements were updated also.


The complete list of changes is at

https://copr-dist-git.fedorainfracloud.org/cgit/osoukup/ctrlp.vim/vim-ctrlp.git/diff/vim-ctrlp.spec

Additionally follows the links to updated COPR and KOJI builds

https://copr.fedorainfracloud.org/coprs/osoukup/ctrlp.vim/build/929280/
https://koji.fedoraproject.org/koji/taskinfo?taskID=35450870

Comment 4 Ondrej Soukup 2019-06-10 11:02:57 UTC
Additionally the following warning is no more produced by rpmlint:

vim-ctrlp.src: W: invalid-url Source0: vim-ctrlp-1.8.0.tar.gz
The value should be a valid, public HTTP, HTTPS, or FTP URL.

Comment 5 Robert-André Mauchin 🐧 2019-06-10 16:44:52 UTC
 - Why -c? This break the build

%setup -cq

 Use:

%autosetup -n ctrlp.vim-%{version}

 - There's no LICENSE file in the archive.

+ cp -pr LICENSE /builddir/build/BUILDROOT/vim-ctrlp-1.80-1.fc31.x86_64/usr/share/licenses/vim-ctrlp
BUILDSTDERR: cp: cannot stat 'LICENSE': No such file or directory
+ :
+ exit 0
BUILDSTDERR: error: File not found: /builddir/build/BUILDROOT/vim-ctrlp-1.80-1.fc31.x86_64/usr/share/licenses/vim-ctrlp/LICENSE

 Add it as another source from GH or consider packaging a snapshot version of the master branch.

%global commit 2e773fd8c7548526853fff6ee2e642eafbbe3d04
%global shortcommit %(c=%%{commit}; echo ${c:0:7})
%global snapshotdate 20190610

Name:           vim-ctrlp
Version:        1.80
Release:        1.%{snapshotdate}git%{shortcommit}%{?dist}
Summary:        Full path fuzzy file, buffer, mru, tag, ... finder for Vim

License:        Vim
URL:            https://github.com/ctrlpvim/ctrlp.vim
Source0:        https://github.com/ctrlpvim/ctrlp.vim/archive/%{commit}/%{name}-%{shortcommit}.tar.gz

[…]

%autosetup -n ctrlp.vim-%{commit}

[…]

* Tue Feb 26 2019 Ondřej Soukup <osoukup> - 1.80-1.20190610git2e773fd

Comment 6 Igor Raits 2019-06-22 16:09:09 UTC
Hey Ondrej,

I sponsored you to the packagers group.

Let me know if you need any assistance with review made by eclipseo.

Comment 7 Ondrej Soukup 2019-06-24 14:52:37 UTC
(In reply to Igor Gnatenko from comment #6)
> Hey Ondrej,
> 
> I sponsored you to the packagers group.
> 
> Let me know if you need any assistance with review made by eclipseo.

Thank you very much Igor! I am also thankful to Robert-André for his reviews. After his last comment I tried to incorporate his suggestions in various ways but I am still just struggling with it without any success. I at least understood some of my previous mistakes like building from my own modified archive instead of the upstream one.

I was first trying to stick with version-directed approach which I was using previously. The farthest I got was to various LICENSE install errors and got stuck with it. I am not sure how to install it. I was not able to find some example or documentation which would work for me.

Now I am investigating the second suggested commit-oriented approach. Simple copy-pasting ended up with invalid spec file so now I am learning it and trying to solve what is the correct working way of using it.

I am doing this my little packaging adventure in my free time and cannot find more than couple of hours for it every week so I am proceeding quite slowly. Moreover, I feel most of my last tries ended up as a waste of time investigating blind ways.

Comment 8 Ondrej Soukup 2019-06-24 18:47:50 UTC
So as soon as I wrote down all these despair here things finally moved forward! I used commit-based approach as suggested by Robert-André - thank you - which works like a charm. I just had to first find out an unrelated typo of mine which made me think it does not work so straightforward...

Here are the changes

https://github.com/osoukup/ctrlp.vim/commit/2afc0feca05dbf86d4fc233b34951056b42b1163

and here are the new builds

https://copr.fedorainfracloud.org/coprs/osoukup/ctrlp.vim/build/944205/
https://koji.fedoraproject.org/koji/taskinfo?taskID=35793933

Thank you both once again!

Comment 9 Robert-André Mauchin 🐧 2019-06-25 14:26:44 UTC
LGTM, package approved.

Comment 10 Ondrej Soukup 2019-06-25 15:15:34 UTC
(In reply to Robert-André Mauchin from comment #9)
> LGTM, package approved.

Thank you for the detailed review and lot of advice!

Comment 11 Gwyn Ciesla 2019-06-28 13:32:54 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/vim-ctrlp

Comment 12 Fedora Update System 2019-07-31 07:49:10 UTC
FEDORA-2019-11d543f05f has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-11d543f05f

Comment 13 Fedora Update System 2019-08-01 03:28:25 UTC
vim-ctrlp-1.80-1.20190610git2e773fd.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-11d543f05f

Comment 14 Fedora Update System 2019-08-09 01:02:43 UTC
vim-ctrlp-1.80-1.20190610git2e773fd.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.


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