Bug 207487 - misc/mc: make libssh optional
Summary: misc/mc: make libssh optional
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Felder
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-25 14:56 UTC by Moritz Wilhelmy
Modified: 2016-03-18 12:05 UTC (History)
2 users (show)

See Also:
woodsb02: maintainer-feedback+


Attachments
turn libssh support into an option (1.02 KB, patch)
2016-02-25 14:56 UTC, Moritz Wilhelmy
no flags Details | Diff
Patch for misc/mc to create new port option SFTP (1.39 KB, patch)
2016-03-17 23:56 UTC, Ben Woods
woodsb02: maintainer-approval+
Details | Diff
QA: Successful poudriere testport logs for misc/mc 4.8.16 with patch on FreeBSD 10.2 amd64 (178.92 KB, text/plain)
2016-03-17 23:57 UTC, Ben Woods
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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!