Bug 205231 - [patch] mail/dovecot2 FTS search - decode2text script / new option with new depends?
Summary: [patch] mail/dovecot2 FTS search - decode2text script / new option with new d...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Adam Weinberger
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-12-10 21:05 UTC by Andrej Ebert
Modified: 2015-12-12 16:49 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (adamw)


Attachments
Change paths from /usr/bin to /usr/local/bin (866 bytes, patch)
2015-12-10 21:05 UTC, Andrej Ebert
no flags Details | Diff
makefile-patch, adding options/depends for FTS attachment decoding (1.98 KB, patch)
2015-12-11 10:49 UTC, Andrej Ebert
no flags Details | Diff
makefile-patch, adding options and post-patching decode2text.sh (2.80 KB, patch)
2015-12-11 19:35 UTC, Andrej Ebert
no flags Details | Diff
makefile-patch, adding options and post-patching decode2text.sh (2.82 KB, patch)
2015-12-11 19:49 UTC, Andrej Ebert
no flags Details | Diff
makefile-patch, adding options and post-patching decode2text.sh (2.82 KB, patch)
2015-12-11 19:59 UTC, Andrej Ebert
no flags Details | Diff
makefile-patch, adding options and post-patching decode2text.sh (2.82 KB, patch)
2015-12-11 20:15 UTC, Andrej Ebert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrej Ebert 2015-12-10 21:05:47 UTC
Created attachment 164089 [details]
Change paths from /usr/bin to /usr/local/bin

While trying to configure FTS search using lucene (including attachments) on my dovecot instance I noticed that the dovecot-supplied script decode2text.sh has some "/usr/bin" paths for needed binaries instead of "/usr/local/bin".
The attached patch fixes that.


Also, I'd like to propose an additional port option with additional dependencies (maybe as a suboption of "Full text search plugins"), something like "Support for Attachment decoding for search", since the decode2text script relies on an pdf2text binary (supplied by either graphics/poppler-utils or graphics/xpdf (Radio button?)) and the catdoc/catppt/xls2csv binaries, supplied by the textproc/catdoc port.
Comment 1 Adam Weinberger freebsd_committer freebsd_triage 2015-12-11 04:40:57 UTC
The patch will definitely be committed, though hard coding /usr/local isn't the best choice. Better to do it in post-patch: with sed.

As for the new option, I'm not a huge fan of options that do nothing except register dependencies. (Unless I'm misreading, I think that's what you're proposing here, right?)

What do you think about adding a sentence to the pkg-message instead?
Comment 2 Andrej Ebert 2015-12-11 10:49:25 UTC
Created attachment 164115 [details]
makefile-patch, adding options/depends for FTS attachment decoding

You're right, sed would be better.
No, you're not misreading.
A pkg-message would work too, of course. 
Although as a user, I like to have packages installed for the sole purpose of fulfilling a dependency marked as such, so, for example "pkg autoremove" would remove them when not needed anymore. But that's more or less a cosmetic/philosophical thing.
I mucked about a little with the makefile, inserting the proposed options, but don't really understand much of ports, so bear with me on the attached patch.
Comment 3 Andrej Ebert 2015-12-11 18:11:41 UTC
Also, decode2text.sh would need to be patched further to use xpdf: the pdf2text binary is being installed to /usr/bin/xpdf (really a symlink to /usr/lib/xpdf) to allow coexistence with poppler-utils. So at least the xpdf option wouldn't be a pure dependency-option anymore.
Comment 4 Andrej Ebert 2015-12-11 19:35:27 UTC
Created attachment 164122 [details]
makefile-patch, adding options and post-patching decode2text.sh

Makefile patch, replacing the paths in decode2text.sh via post-patch depending on selected options an adding proposed options to the makefile.
Comment 5 Andrej Ebert 2015-12-11 19:49:53 UTC
Created attachment 164123 [details]
makefile-patch, adding options and post-patching decode2text.sh

Forgot to define the dependencies as RUN_DEPENDS.
Comment 6 Andrej Ebert 2015-12-11 19:59:41 UTC
Created attachment 164126 [details]
makefile-patch, adding options and post-patching decode2text.sh

Fix typo.
Comment 7 Andrej Ebert 2015-12-11 20:15:41 UTC
Created attachment 164128 [details]
makefile-patch, adding options and post-patching decode2text.sh

Sorry for the spam, fix another typo, should be ok now.
Comment 8 commit-hook freebsd_committer freebsd_triage 2015-12-12 16:47:16 UTC
A commit references this bug:

Author: adamw
Date: Sat Dec 12 16:47:13 UTC 2015
New revision: 403608
URL: https://svnweb.freebsd.org/changeset/ports/403608

Log:
  Update dovecot2 to 2.2.21, and bump PORTREVISION in consumers.

  While here, fix up some paths in the decode2text plugin, and add a note
  to pkg-message about how to make it work. [1]

  Changes:
   - doveadm mailbox list (and some others) were broken in v2.2.20
   - director: Fixed making backend changes when running with only a
     single director server.
   - virtual plugin: Fixed crash when trying to open nonexistent
     autocreated backend mailbox.

  PR:		205231 [1]
  Submitted by:	andrej@ebert.su

Changes:
  head/mail/dovecot2/Makefile
  head/mail/dovecot2/distinfo
  head/mail/dovecot2/files/patch-src_plugins_fts_decode2text.sh
  head/mail/dovecot2/files/pkg-message.in
  head/mail/dovecot2-antispam-plugin/Makefile
  head/mail/dovecot2-pigeonhole/Makefile
Comment 9 Adam Weinberger freebsd_committer freebsd_triage 2015-12-12 16:48:48 UTC
Wow, good work on those patches!

I went with a bit of a simpler solution, teaching the script to check the xpdf location and the poppler location, and then substituting with a single sed command.
Comment 10 Adam Weinberger freebsd_committer freebsd_triage 2015-12-12 16:49:51 UTC
Thanks for all your work on this and for filing this PR. If you find anywhere else that's got hardcoded paths like that, please file another one!