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 1362487 - Review Request: arcanist - A command line interface to Phabricator
Summary: Review Request: arcanist - A command line interface to Phabricator
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Andreas Schneider
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1362490
Blocks: 1362491
TreeView+ depends on / blocked
 
Reported: 2016-08-02 11:01 UTC by Jeroen van Meeuwen
Modified: 2022-09-16 10:25 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-07-10 08:10:25 UTC
Type: ---
Embargoed:
asn: fedora-review?


Attachments (Terms of Use)

Description Jeroen van Meeuwen 2016-08-02 11:01:45 UTC
Spec URL: https://kanarip.fedorapeople.org/phabricator/arcanist.spec
SRPM URL: https://kanarip.fedorapeople.org/phabricator/arcanist-20160727.gitf1c45a3-1.1.el7.src.rpm
Description: A command-line interface to Phabricator
Fedora Account System Username: kanarip

Comment 1 Jared Smith 2016-08-03 14:33:03 UTC
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

Comment 3 Jared Smith 2016-08-03 15:34:44 UTC
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.

Comment 4 Jeroen van Meeuwen 2016-08-09 11:51:41 UTC
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.

Comment 5 Jeroen van Meeuwen 2016-08-09 11:57:16 UTC
(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.

Comment 7 Tim Flink 2016-08-11 11:13:05 UTC
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).

Comment 8 Jeroen van Meeuwen 2016-08-16 07:31:19 UTC
It requires /usr/bin/php, and thus whatever provides that, nay?

Comment 9 Tim Flink 2016-09-06 18:45:52 UTC
I don't see the requires on /usr/bin/php in the specfile, though

Comment 10 Kamil Páral 2016-09-14 10:45:06 UTC
(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

Comment 11 Tim Flink 2016-09-15 20:15:43 UTC
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.

Comment 13 Tim Flink 2016-09-23 19:06:26 UTC
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

Comment 14 Jared Smith 2016-09-23 20:04:51 UTC
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

Comment 16 Jared Smith 2016-09-26 14:32:09 UTC
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)?

Comment 17 Mamoru TASAKA 2016-09-26 14:54:35 UTC
(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)

Comment 18 Tim Flink 2016-11-28 15:39:33 UTC
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?

Comment 19 Jared Smith 2016-11-29 18:11:57 UTC
Yes, in the interest in moving forward, I'm happy to accept it :-)

Comment 22 Ling Li 2017-01-25 19:13:51 UTC
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.

Comment 23 Andreas Schneider 2017-06-20 15:57:57 UTC
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

Comment 24 Package Review 2020-07-10 00:54:59 UTC
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.


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