Bug 1362487

The spec file doesn't match the version of of the spec file used to build the SRPM.

You should set the Version: tag to 0 and the Release: tag to 0.1.20160727.git%{git_short_version_hash}%{dist} 

(See the Pre-release section of https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#NonNumericRelease for more information on versioning of pre-releases.)

Use the following URL for Source0: https://github.com/phacility/arcanist/archive/%{git_full_version_hash}.tar.gz#/arcanist-%{git_short_version_hash}.tar.gz

Spec URL: https://kanarip.fedorapeople.org/phabricator/arcanist.spec
SRPM URL: https://kanarip.fedorapeople.org/phabricator/arcanist-0-0.20160727.gitf1c45a3.el7.src.rpm

Needs libphutil, so I can't yet complete the review.  In the meantime, here are some rpmlint warnings.

Rpmlint
-------
Checking: arcanist-0-0.20160727.gitf1c45a3.fc26.noarch.rpm
          arcanist-0-0.20160727.gitf1c45a3.fc26.src.rpm
arcanist.noarch: E: explicit-lib-dependency libphutil
arcanist.noarch: W: no-documentation
arcanist.noarch: E: zero-length /usr/share/arcanist/src/repository/parser/__tests__/mercurial/branches-empty.txt
arcanist.noarch: E: non-executable-script /usr/share/arcanist/src/lint/linter/__tests__/chmod/shebang.lint-test 644 /bin/bash 
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/src/lint/linter/xhpast/rules/__tests__/inline-html/inline-html.lint-test /usr/bin/env php
arcanist.noarch: E: non-executable-script /usr/share/arcanist/src/lint/linter/xhpast/rules/__tests__/inline-html/inline-html.lint-test 644 /usr/bin/env php
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/bin/arc /usr/bin/env bash
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/scripts/arcanist.php /usr/bin/env php
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/scripts/breakout.py /usr/bin/env python2
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/scripts/hgdaemon/hgdaemon_client.php /usr/bin/env php
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/src/lint/linter/xhpast/rules/__tests__/php-open-tag/php-tags-script.lint-test /usr/bin/local/php 
arcanist.noarch: E: non-executable-script /usr/share/arcanist/src/lint/linter/xhpast/rules/__tests__/php-open-tag/php-tags-script.lint-test 644 /usr/bin/local/php 
arcanist.noarch: E: wrong-script-interpreter /usr/share/arcanist/scripts/hgdaemon/hgdaemon_server.php /usr/bin/env php
arcanist.noarch: W: no-manual-page-for-binary arc
arcanist.src: W: invalid-url URL: http://www.phabricator.com/docs/arcanist/ <urlopen error [Errno -2] Name or service not known>
arcanist.src: W: file-size-mismatch arcanist-f1c45a3.tar.gz = 501141, https://github.com/phacility/arcanist/archive/f1c45a3323ae20eefe29c0a22c7923fe8b151bbf.tar.gz#/arcanist-f1c45a3.tar.gz = 500723
2 packages and 0 specfiles checked; 12 errors, 4 warnings.

Note while working on libphutil, that to have rpmlint and builds from source succeed, the Source0 URL will likely just need to be:

> https://github.com/phacility/arcanist/archive/%{git_full_version_hash}.tar.gz

Adjust gen-tar.sh to reflect this.

Also note to require phabricator(libphutil) in an attempt to silence rpmlint.

(In reply to Jeroen van Meeuwen from comment #4)
> Note while working on libphutil, that to have rpmlint and builds from source
> succeed, the Source0 URL will likely just need to be:
> 
> > https://github.com/phacility/arcanist/archive/%{git_full_version_hash}.tar.gz
> 
> Adjust gen-tar.sh to reflect this.
> 

That's wrong, I mean to use --content-disposition option to wget instead and use the full version hash instead of the short version hash for the trailing part.

Spec URL: https://kanarip.fedorapeople.org/phabricator/arcanist.spec
SRPM URL: https://kanarip.fedorapeople.org/phabricator/arcanist-20160806.gitc9337c2-1.fc26.noarch.rpm

Doesn't arcanist need php-cli? I know that some folks have had problems using arcanist until php-cli was installed (if it wasn't installed already).

It requires /usr/bin/php, and thus whatever provides that, nay?

I don't see the requires on /usr/bin/php in the specfile, though

(In reply to Tim Flink from comment #7)
> Doesn't arcanist need php-cli? I know that some folks have had problems
> using arcanist until php-cli was installed (if it wasn't installed already).

https://phab.qadevel.cloud.fedoraproject.org/T796

I installed the libphutil and arcanist packages from these reviews into a clean f26 chroot and php-cli is installed with arcanist with no issue. If I use the older packages that are referenced in the ticket that Kamil linked to, there is a problem.

Therefore I conclude that there is no longer an issue with the requires and the review can go ahead.

Spec URL: https://tflink.fedorapeople.org/packages/phabricator-review/arcanist/arcanist.spec
SRPM URL: https://tflink.fedorapeople.org/packages/phabricator-review/arcanist/arcanist-20160806.gitc9337c2-1.fc26.src.rpm

Note that the content is still the same, I just rebuilt the SRPM from sources because the link in c#6 goes to a .noarch instead of a .src.rpm

The version and release tags are still wrong.  As I mentioned in comment number 1 and in person at Flock, you should set the Version: tag to 0 and the Release: tag to 0.1.20160806.git%{git_short_version_hash}%{dist} 

If you were to do a second build with that same git hash, it would then be 0.2.20160806.git%{git_short_version_hash}%{dist}, and so on.

This way, if/when a version 1.0 comes out, it will be greater than version 0.x.whatever.

See the Pre-release section of https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#NonNumericRelease for more information on versioning of pre-releases, especially the example of kismet in the pre-release section of that page

Spec URL: https://tflink.fedorapeople.org/packages/phabricator-review/arcanist/arcanist.spec
SRPM URL: https://tflink.fedorapeople.org/packages/phabricator-review/arcanist/arcanist-0.0-2.20160806.gitc9337c2.fc26.src.rpm

This package is getting closer to being ready for approval.  The remaining outstanding items are:

1) This package fails to install, because of the missing dependency on libphutil.

2) The spec file should use "%global" instead of "%define", unless you can justify the reason why you need to use "%define".

3) It's not a huge deal, but rpmlint is complaining that the bash completion file isn't marked as %config(noreplace) instead of just %config.  Have you thought about whether or not you'd want this file to get replaced with a new version (if someone had altered the file)?

(In reply to Jared Smith from comment #16)
> 3) It's not a huge deal, but rpmlint is complaining that the bash completion
> file isn't marked as %config(noreplace) instead of just %config.  Have you
> thought about whether or not you'd want this file to get replaced with a new
> version (if someone had altered the file)?

Well, now bash completion directory is %_datadir/bash-completion/completions , not %{_sysconfdir}/bash_completion.d (the latter is deprecated).
You can check what

$ pkg-config --variable=completionsdir bash-completion

returns (and as bash completion files is now not under %_sysconfdir, no %config should be added)

Sorry for the delay on this, got very distracted.

(In reply to Jared Smith from comment #14)
> The version and release tags are still wrong.  As I mentioned in comment
> number 1 and in person at Flock, you should set the Version: tag to 0 and
> the Release: tag to 0.1.20160806.git%{git_short_version_hash}%{dist} 

After looking at this again, I get what you're saying but I'd argue that's not quite what the versioning guidelines say.

https://fedoraproject.org/wiki/Packaging:Versioning#Pre-Release_packages

The way I'm reading that, the following should be fine so long as the version starts with a 0 in case upstream ever does a proper release:

Version: 0.20160806.git%{git_short_version_hash}
Release: 1


Would this version/release scheme be acceptable?

Yes, in the interest in moving forward, I'm happy to accept it :-)

Spec URL: https://kanarip.fedorapeople.org/phabricator/arcanist.spec
SRPM URL: https://kanarip.fedorapeople.org/phabricator/arcanist-0.20161022.gite17fe43-2.el7.src.rpm

Spec URL: https://kanarip.fedorapeople.org/phabricator/arcanist.spec
SRPM URL: https://kanarip.fedorapeople.org/phabricator/arcanist-0.20161022.gite17fe43-7.el7.src.rpm

When using the libphutil package (from https://kanarip.fedorapeople.org/phabricator/), arc complains about 

Exception
[cURL/77] (https://company.com/api/user.whoami) The SSL CA Bundles that we tried to use could not be read or are not formatted correctly.
(Run with `--trace` for a full exception trace.)

If I added resources/ into /usr/share/libphutil, then everything works fine.

I don't know PHP/libphutil enough and couldn't tell if it's something wrong in my system or in the RPM.  So, just for your information.

I'm interested in this package and have created a spec file, works fine on fedora 26

Spec URL: https://xor.cryptomilk.org/rpm/arcanist/arcanist.spec
SRPM URL: https://xor.cryptomilk.org/rpm/arcanist/arcanist-0.0-0.1.20170605git0c53a35.fc26.src.rpm

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.