Page MenuHomeFreeBSD

Update phabricator to the latest version
AbandonedPublic

Authored by eadler on Jun 24 2015, 3:13 AM.

Details

Reviewers
grembo
koobs
mat
Test Plan

nada

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

eadler updated this revision to Diff 6416.Jun 24 2015, 3:13 AM
eadler retitled this revision from to Update phabricator to the latest version.
eadler updated this object.
eadler edited the test plan for this revision. (Show Details)
eadler added reviewers: koobs, grembo.
eadler added a subscriber: adrian.
koobs requested changes to this revision.Jun 24 2015, 4:30 AM
koobs edited edge metadata.

Looks good to me except for inline comments.

I can't approve without poudriere / portlint logs, especially given the substantial pkg-plist changes

devel/phabricator/Makefile
12

Intentional? No more LICENSE_FILE in WRKSRC?

18

Tab alignment

This revision now requires changes to proceed.Jun 24 2015, 4:30 AM
grembo requested changes to this revision.Jun 24 2015, 3:48 PM
grembo edited edge metadata.

Please fix distinfo of arcanist and test building all three ports.
(I'll install and test those versions @work to see if they are ok otherwise)

devel/arcanist/distinfo
2

I assume this should be phacility-arcanist-20150623-...

===>  License APACHE20 accepted by the user
===>   php5-arcanist-20150623 depends on file: /usr/local/sbin/pkg - found
=> phacility-arcanist-20150623-b697a3b_GH0.tar.gz is not in /basejail/usr/ports/devel/arcanist/distinfo.
=> Either /basejail/usr/ports/devel/arcanist/distinfo is out of date, or
=> phacility-arcanist-20150623-b697a3b_GH0.tar.gz is spelled incorrectly.
*** Error code 1
grembo added inline comments.Jun 24 2015, 3:55 PM
devel/libphutil/distinfo
2

Same problem here, should be 20150623

eadler marked 2 inline comments as done.Jun 24 2015, 9:13 PM
eadler added inline comments.
devel/arcanist/distinfo
2

This works for me?

[10010 eax@fbsdvm   .../ports/devel/arcanist !2!]%make makesum
===>  License APACHE20 accepted by the user
===>   php5-arcanist-20150623 depends on file: /usr/local/sbin/pkg - found
===> Fetching all distfiles required by php5-arcanist-20150623 for building
devel/phabricator/Makefile
12

correct.

18

looks fine to me:

eadler updated this revision to Diff 6445.Jun 24 2015, 9:13 PM
eadler edited edge metadata.
eadler marked 2 inline comments as done.

per mva, koobs

After fixing libphutil/distinfo this installed fine. I rolled it out at work yesterday and so far it seems to work ok.

devel/arcanist/distinfo
2

I applied the raw patch (download raw diff) and that contained:

-SHA256 (phacility-arcanist-20150602-8c589f1_GH0.tar.gz) = f00c87d46654d4b5d333e031d35cf923b7f0168e5fc6a2440f7688ffd1d4e27c
-SIZE (phacility-arcanist-20150602-8c589f1_GH0.tar.gz) = 458899
+SHA256 (phacility-arcanist-20150602-b697a3b_GH0.tar.gz) = f98715b9c4aaae44ff1ce2649ee3bd9819053669597251637a55d24a12530244
+SIZE (phacility-arcanist-20150602-b697a3b_GH0.tar.gz) = 460003
Index: devel/arcanist/pkg-plist

So either I missed an iteration or it's (ironically) a phabricator problem.

devel/libphutil/distinfo
2

This is definitely broken, also in the latest iteration.

devel/phabricator/Makefile
18

Unfortunately Phabricator is not good at indenting, is there anything we can do to fix this? Indentation should be ok here, like you said.

mat accepted this revision.Jun 26 2015, 8:28 AM
mat added a reviewer: mat.
mat added a subscriber: mat.

If it builds, please commit this.

grembo added a subscriber: bapt.Jun 26 2015, 8:42 AM

@mat this won't build as long as devel/libphutil/distinfo is broken (see my comment above). Accept Revision shouldn't have an "if it works" comment in it, but mean that some actual review happened (well, AFAIK).

Also note that this version won't include the workaround for curl 7.43 that broke arcanist.

@bapt, @mat: Should I just fix these three for @eadler, merge latest versions from today and commit it?

mat added a comment.Jun 26 2015, 8:55 AM
In D2894#56654, @grembo wrote:

@mat this won't build as long as devel/libphutil/distinfo is broken (see my comment above). Accept Revision shouldn't have an "if it works" comment in it, but mean that some actual review happened (well, AFAIK).

Then, apply the patch and run make makesum, there fixed it for you.

Also note that this version won't include the workaround for curl 7.43 that broke arcanist.
@bapt, @mat: Should I just fix these three for @eadler, merge latest versions from today and commit it?

Yes, please, make it work, it's been broken too long.

mat added a comment.Jun 26 2015, 8:55 AM

Also, I feel these ports should be maintained by phabric-admin@.

I'm more than happy to pass those ports on to you @mat or phabric-admin, please make sure you update and test devel/phabrictor though (as phabric-admin@ runs a heavily customized version of phabricator).

By the way, there are no bug reports in bugs.freebsd.org for any of the affected packages (devel/libphutil, devel/arcanist or ftp/curl, which is a big part of the problem).

koobs added a comment.Jun 26 2015, 9:41 AM

Created blocking bug for updating phabricator (linked to this review)

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=201121

@koobs thanks, I'm building new versions of those packages right now, I'll test them and commit shortly.

grembo abandoned this revision.Jun 26 2015, 10:17 AM

Updated to versions a of 20150626, therefore this review is obsolete now => Abandon Revision.

Date: Fri Jun 26 10:12:45 UTC 2015
New revision: 390625
URL: https://svnweb.freebsd.org/changeset/ports/390625

@grembo @mat sorry about the delay in fixing/committing this :\

@eadler no need to apologize, thanks for your effort.

To wrap this up (as there has been some confusion):

The problem with arcanist was caused by a problem in curl 7.43.0, a workaround to libphutil was released on June 24th (one day after this review was opened), the whole discussion of the problem can be found here: https://secure.phabricator.com/T8654

The curl maintainers proposed a patch to fix this and then committed it.

I opened PR 201147 that incorporates the patch to ftp/curl from upstream. In my opinion this would've been the correct way to fix this in the first place, instead of rushing to release a workaround version of libphutil and not fixing the issue for other uses of libcurl.