Bug 220926

Summary: sysutils/linux-crashplan: Update to 4.8.3
Product: Ports & Packages Reporter: Nuno Subtil <subtil>
Component: Individual Port(s)Assignee: Richard Gallamore <ultima>
Status: Closed FIXED    
Severity: Affects Some People CC: dan, miwi, subtil, sze.david, ultima
Priority: --- Keywords: patch-ready
Version: LatestFlags: subtil: maintainer-feedback+
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219127
Attachments:
Description Flags
Update sysutils/linux-crashplan to version 4.8.3
none
linux-crashplan.diff
subtil: maintainer-approval-
Patch proposal for Crashplan 4.8.3
subtil: maintainer-approval+
linux-crashplan.diff subtil: maintainer-approval+

Description Nuno Subtil 2017-07-22 19:06:32 UTC
Created attachment 184599 [details]
Update sysutils/linux-crashplan to version 4.8.3

Supersedes bug 219127 (upstream has since been updated).
Comment 1 Dan Moutal 2017-07-23 20:53:02 UTC
I am probably doing something wrong but I can't seem to apply this patch:

# patch -i ../crashplan-patch 
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sysutils/linux-crashplan/Makefile b/sysutils/linux-crashplan/Makefile
|index 8284e8946395..f81c7b192ab3 100644
|--- a/sysutils/linux-crashplan/Makefile
|+++ b/sysutils/linux-crashplan/Makefile
--------------------------
Patching file Makefile using Plan A...
patch: **** malformed patch at line 7:


I was able to update the patch from bug #219127 and that seemed to work without issue
I can upload that if needed
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-24 02:23:53 UTC
Thank you for your update Nuno. Please use the maintainer-approval flag (set to +) on attachments to signify maintainer submissions/approval. Attachment -> Details -> maintainer-approval [+]
Comment 3 Nuno Subtil 2017-07-25 00:40:11 UTC
Dan, were you trying to apply this patch on top of the old one in bug 219127?
Comment 4 Dan Moutal 2017-07-25 07:55:12 UTC
Nope, on the version of the port currently in the Ports tree. 4.7_1

I think my problem is my lack of familiarity with patch. What is the correct patch command to apply this patch?
Comment 5 David Sze 2017-07-30 13:21:00 UTC
(In reply to Dan Moutal from comment #4)

The patch has DOS-style line endings - if you convert it to UNIX-style you should have better luck applying it.
Comment 6 Dan Moutal 2017-08-01 06:28:33 UTC
Making some progress, but still having issues
Converting the line ending to UNIX-style helps but the patch only partially applies

$ sudo patch -i ../crashplan.txt
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sysutils/linux-crashplan/Makefile b/sysutils/linux-crashplan/Makefile
|index 8284e8946395..f81c7b192ab3 100644
|--- a/sysutils/linux-crashplan/Makefile
|+++ b/sysutils/linux-crashplan/Makefile
--------------------------
Patching file Makefile using Plan A...
Hunk #1 failed at 2.
Hunk #2 failed at 35.
2 out of 2 hunks failed--saving rejects to Makefile.rej
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sysutils/linux-crashplan/distinfo b/sysutils/linux-crashplan/distinfo
|index c7486cb3b96a..7dcd48bc95bc 100644
|--- a/sysutils/linux-crashplan/distinfo
|+++ b/sysutils/linux-crashplan/distinfo
--------------------------
Patching file distinfo using Plan A...
Hunk #1 succeeded at 1.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sysutils/linux-crashplan/pkg-plist b/sysutils/linux-crashplan/pkg-plist
|index 9d8382e78772..4c81d8c7e95b 100644
|--- a/sysutils/linux-crashplan/pkg-plist
|+++ b/sysutils/linux-crashplan/pkg-plist
--------------------------
Patching file pkg-plist using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 succeeded at 32.
Hunk #3 succeeded at 71.
Hunk #4 succeeded at 133.
done




If other people are having better luck then likely the issue is with my lack of patching ability rather than a problem with the patch. More importantly if the patched port works correctly we probably shouldn't let my difficulty in patching stop this patch from moving forward
Comment 7 Richard Gallamore freebsd_committer 2017-08-06 02:25:02 UTC
Created attachment 185069 [details]
linux-crashplan.diff

I was about to test this, however jdk8 needs to be downloaded and I was unable to do so. I found some items that needed to be corrected and also took out some extra steps that aren't required.
Comment 8 Richard Gallamore freebsd_committer 2017-08-06 02:26:39 UTC
Also, i'm pretty sure etc/rc.d/crashplan will need to be removed from the pkg-plist,  but as I said I can't test this so not positive.
Comment 9 Richard Gallamore freebsd_committer 2017-08-06 02:28:35 UTC
(In reply to Dan Moutal from comment #1)
With git patches, patch usually needs to strip the first */, this is done by adding the -p1 option.
Comment 10 Dan Moutal 2017-08-06 17:03:37 UTC
Thanks for the tip regarding git patches

I did try the latest patch and while the patch was successful I wasn't able to build the crashplan package

Here is the log leading up to the error:

--------------------------------------------------------------------------------
--  Phase: package
--------------------------------------------------------------------------------
===>  Building package for linux-crashplan-4.8.3
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_de.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_en_GB.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_en_US.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_es.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_es_ES.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_fr.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_it.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_ja.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_ko.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_nl.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_no.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_pt_BR.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_sv.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_th.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_zh.properties:No such file or directory
pkg-static: Unable to access file /construction/xports/sysutils/linux-crashplan/work/stage/usr/local/share/crashplan/conf/txt_zh_TW.properties:No such file or directory
pkg-static: duplicate file listing: /usr/local/etc/rc.d/crashplan, ignoring
*** Error code 1

Stop.
make: stopped in /xports/sysutils/linux-crashplan
Comment 11 Dan Moutal 2017-08-06 17:08:56 UTC
Created attachment 185095 [details]
Patch proposal for Crashplan 4.8.3

This is the patch I have been using successfully for the past few weeks

I took the batch from bug 219127 and updated it so it grabs Crashplan 4.83. It seems to work without any issues.

I would be happy to get someone with more experience to take a look and confirm that this wont do anything inappropriate.
Comment 12 Nuno Subtil 2017-08-06 19:22:51 UTC
I get the same results that Dan reported with Richard's patch. It doesn't seem to be working correctly. I'm flagging this down for now.

Dan's patch seems identical to the original one that I posted, modulo pkg-plist changes. It builds and installs fine on my system as well.

It seems like this boils down to issues applying that patch, which could be explained by the fact that I built it on the git tree (besides the DOS line endings --- sorry!). This causes the Makefile to not have the SVN version header, which unfortunately shows up in the diff and causes patch to reject those hunks.

As a path forward, I'd suggest Richard to review the pkg-plist deltas between the two patches and suggest a course of action. It's not clear to me why etc/rc.d/crashplan should not be in pkg-plist --- is there some guidance on this?

I'll wait a few days to get some feedback on this. If I don't hear back, I'll approve one of the existing patches to attempt once again to get some closure on updating this port.
Comment 13 Richard Gallamore freebsd_committer 2017-08-06 22:50:40 UTC
Comment on attachment 185069 [details]
linux-crashplan.diff

Sorry about that, I'll just set obsolete. Wish I could test further.
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2017-08-07 09:12:28 UTC
Comment on attachment 185095 [details]
Patch proposal for Crashplan 4.8.3

The 'wait for feedback' bit is also a little ambiguous/confusing. What feedback is being sought, by who and from who?

comment 12 indicates that 'Dans patch works', and is an update to 4.8.3 as indicated by this issue summary. Is there any reason why it cannot be approved and committed?
Comment 15 Nuno Subtil 2017-08-09 02:36:29 UTC
The intent was to hold off on submitting anything while we were waiting for feedback from Richard on the pkg-plist issue. Sorry if the maintainer flags became confusing.

Since no further issues have been raised, I agree with submitting Dan's patch.
Comment 16 Richard Gallamore freebsd_committer 2017-08-09 02:56:59 UTC
(In reply to Nuno Subtil from comment #15)
Ah, the patch you approved doesn't include the file I was questioning and looks correct as far as I can tell without doing QA myself. I'll try to download java again.
Comment 17 Kubilay Kocak freebsd_committer freebsd_triage 2017-08-09 03:48:18 UTC
@Nuno, Independent of patch approval, the patch is still pending QA (poudriere) confirmation. It's preferable that this come from the maintainer (confirming runtime, not just packaging success)
Comment 18 Dan Moutal 2017-08-09 03:52:16 UTC
I am not the maintainer but can confirm Cashplan works after this patch without issues. I have been running it since the 23rd without any noticeable issues
Comment 19 Richard Gallamore freebsd_committer 2017-08-09 04:07:04 UTC
Created attachment 185182 [details]
linux-crashplan.diff

Still couldn't download java, however I took another look at the port and no build is required so I just commented java for QA.

The previous patch had missing files in pkg-plist and can't be committed because of this. Attached I fixed all outstanding issues and also cleaned up the port. Don't think any changes I made will cause issues, however can someone please do one more test on this patch and verify everything works properly?
Comment 20 Nuno Subtil 2017-08-09 04:29:13 UTC
I'm not sure I understand what we're trying to achieve with the new changes, but I'm not going to let that prevent this port from finally being updated.

I'll test the new patch as soon as I have a chance, unless Dan gets to it first. The pkg-plist changes look OK to me (they're nearly identical to the original patch, modulo the rc.d script). I didn't dig through the Makefile changes too deeply, but it looks as though we're mostly renaming and re-sorting variable declarations, which I don't have a strong opinion on.

Richard, if there's some style guide that we're not adhering to, feel free to post a link for future reference. I'd like to make sure we can do whatever due diligence is required ahead of time whenever we have to update this port again, but I couldn't find much concrete style guidance in the handbook.
Comment 21 Dan Moutal 2017-08-09 04:40:27 UTC
Just finished some basic tests.
Looks like Crashplan runs without any issues
Comment 22 Richard Gallamore freebsd_committer 2017-08-09 05:03:05 UTC
(In reply to Nuno Subtil from comment #20)
Yes most of the changes are simply to adhere to the Makefile's port ordering[1]. CRASHDIR variable was removed because it is set to the same variable as DATADIR which is ${PREFIX}/share/${PORTNAME}. The rest is more or less the same as the original patch. The only real issue my first patch had was that I was assuming that ${DATADIR}/lang directory was created during install which is actually also created during extraction of the next command. Also plist items of course.


[1] https://www.freebsd.org/doc/en/books/porters-handbook/book.html#porting-order
Comment 23 Nuno Subtil 2017-08-09 05:10:38 UTC
Comment on attachment 185182 [details]
linux-crashplan.diff

Looks like we're good to go then. Thanks Dan and Richard for the feedback!
Comment 24 commit-hook freebsd_committer 2017-08-11 07:21:52 UTC
A commit references this bug:

Author: ultima
Date: Fri Aug 11 07:21:44 UTC 2017
New revision: 447734
URL: https://svnweb.freebsd.org/changeset/ports/447734

Log:
  * Updated to 4.8.3
  * Cleanup Makefile

  PR:		220926
  Submitted by:	Nuno Subtil (maintainer)
  Reviewed by:	lifanov (mentor), matthew (mentor)
  Approved by:	matthew (mentor)
  Differential Revision:	https://reviews.freebsd.org/D11931

Changes:
  head/sysutils/linux-crashplan/Makefile
  head/sysutils/linux-crashplan/distinfo
  head/sysutils/linux-crashplan/pkg-plist
Comment 25 Richard Gallamore freebsd_committer 2017-08-11 07:28:09 UTC
Committed, thanks everyone.