Bug 207487

Summary: misc/mc: make libssh optional
Product: Ports & Packages Reporter: Moritz Wilhelmy <mw+freebsd>
Component: Individual Port(s)Assignee: Mark Felder <feld>
Status: Closed FIXED    
Severity: Affects Only Me CC: feld, woodsb02
Priority: --- Flags: woodsb02: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
turn libssh support into an option
none
Patch for misc/mc to create new port option SFTP
woodsb02: maintainer-approval+
QA: Successful poudriere testport logs for misc/mc 4.8.16 with patch on FreeBSD 10.2 amd64 none

Description Moritz Wilhelmy 2016-02-25 14:56:08 UTC
Patch attached, but I don't know if it's okay. I don't really want to install libssh.
Comment 1 Moritz Wilhelmy 2016-02-25 14:56:58 UTC
Created attachment 167400 [details]
turn libssh support into an option
Comment 2 Ben Woods freebsd_committer 2016-02-25 21:36:04 UTC
Hi Moritz, thanks for your patch! :)
It looks mostly good, but I have 2 minor improvements if that is ok. They are both based on the fact that according to the mc project documentation, libssh is only required for supporting SFTP as a virtual file system. [1]

1. The options should be named SFTP rather than LIBSSH, and the description should be something along the lines of "Support for SFTP (via libssh)"

2. In addition to setting the libssh dependency, this option should also cause the configure script to be called with either --enable-vfs-sftp or --disable-vfs-sftp [2]. The following line should be added below the LIBSSH_LIB_DEPENDS line to achieve this:
SFTP_CONFIGURE_ENABLE=	vfs-sftp


Do you think you would be able to resubmit your patch with these changes?

Lastly, if you do have any way to test building this port with the new option enabled, and then again with it disabled, it is always good to attach any evidence that it works. Thanks again :)

 [1] http://www.midnight-commander.org/browser/doc/INSTALL#L23
 [2] http://www.midnight-commander.org/browser/doc/INSTALL#L179
Comment 3 Ben Woods freebsd_committer 2016-03-13 23:11:33 UTC
Hi Moritz,
Just wondering if you have any thoughts on my previous response? I think it is a good idea, and your patch only needs a few small improvements to be ready for commit. Do you think you will be able to submit an updated version?
Thanks again,
Ben
Comment 4 Moritz Wilhelmy 2016-03-15 13:09:27 UTC
Hi Ben,

Thanks for reminding me, I would have forgotten all about it, to be
honest.

I think we need to add '--disable-vfs-sftp' to the configure options, if
the SFTP option is not enabled. Do you know whether something like
NO_SFTP_CONFIGURE_DISABLE is possible?

Best regards,

Moritz
Comment 5 Ben Woods freebsd_committer 2016-03-17 23:56:40 UTC
Created attachment 168348 [details]
Patch for misc/mc to create new port option SFTP

I have updated the patch provided by Moritz based on the recommendations I provided above. Note that the SFTP_CONFIGURE_ENABLE will automatically add either --enable-vfs-sftp or --disable-vfs-sftp depending on the status of the SFTP option. Refer to the FreeBSD porter's handbook for details on this option helper:
https://www.freebsd.org/doc/en/books/porters-handbook/makefile-options.html#options-configure_enable
Comment 6 Ben Woods freebsd_committer 2016-03-17 23:57:52 UTC
Created attachment 168349 [details]
QA: Successful poudriere testport logs for misc/mc 4.8.16 with patch on FreeBSD 10.2 amd64
Comment 7 commit-hook freebsd_committer 2016-03-18 12:04:56 UTC
A commit references this bug:

Author: feld
Date: Fri Mar 18 12:04:32 UTC 2016
New revision: 411345
URL: https://svnweb.freebsd.org/changeset/ports/411345

Log:
  misc/mc: Multiple fixes

  - Fix subshell functionality for csh users
  - Make SFTP an option, enabled by default
  - Fix SIGILL on FreeBSD 9.x
  - Fix perl ls helper with fish shell

  PR:		207487, 208027, 208102, 208104

Changes:
  head/misc/mc/Makefile
  head/misc/mc/files/patch-upstreamticket2742-detect-csh-as-tcsh-by-name.patch
  head/misc/mc/files/patch-upstreamticket3611-fish-fix-perl-ls-helper.patch
  head/misc/mc/files/patch-upstreamticket3617-mc_open-handle-varargs-mode_t-promotion-issue--v2.patch
Comment 8 Mark Felder freebsd_committer 2016-03-18 12:05:33 UTC
Committed, thanks!