Bug 225843 - ports-mgmt/portmaster: -v pollutes gen_dep_list output
Summary: ports-mgmt/portmaster: -v pollutes gen_dep_list output
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Stefan Eßer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-12 03:47 UTC by John Hein
Modified: 2018-02-13 17:46 UTC (History)
1 user (show)

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


Attachments
[patch] direct export_flavor() output to stderr to avoid polluting gen_dep_list() output (566 bytes, patch)
2018-02-12 03:47 UTC, John Hein
no flags Details | Diff
[patch] [alt ver] direct export_flavor() output to stderr to avoid polluting gen_dep_list() output (768 bytes, patch)
2018-02-12 04:04 UTC, John Hein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2018-02-12 03:47:23 UTC
Created attachment 190535 [details]
[patch] direct export_flavor() output to stderr to avoid polluting gen_dep_list() output

If you use '-v', that triggers a message in export_flavor() that pollutes the output of gen_dep_list().

Here's an example of what can happen as a result.  In this example assume electrum and one of its python dependencies (say security/py-trezor@py36) is not installed:

% /usr/local/sbin/portmaster -v finance/electrum
===>>> Port directory: /usr/ports/finance/electrum
 .
 .
===>>> The following actions will be taken if you choose to proceed:
        Install finance/electrum
        Install security/py-trezor@py36

===>>> Proceed? y/n [y] y


===>>> Starting build for finance/electrum <<<===

===>>> Starting check for build dependencies
===>>> Gathering dependency list for finance/electrum from ports
===>>> Starting dependency check
===>>> Checking dependency: ===>>>

===>>> Cannot cd to ===>>>
===>>> Aborting update


The problem is that gen_dep_list() calls export_flavor() and there is a pm_v() call in export_flavor() which outputs to stdout which pollutes the list that is output by gen_dep_list().  If you run portmaster with -x (sh -x /usr/local/sbin/portmaster ...), you can see what is happening.

The fix I attached redirects the export_flavor() output in gen_dep_list() to stderr.

You could also redirect the pm_v output in export_flavor() to stderr.  Maybe that's better, but the other export_flavor() calls in portmaster are not vulnerable to this problem (they don't need stdout to be clean).
Comment 1 John Hein 2018-02-12 04:04:19 UTC
Created attachment 190536 [details]
[patch] [alt ver] direct export_flavor() output to stderr to avoid polluting gen_dep_list() output

Here's an alternate patch that puts the stderr redirect in export_flavor().

Maybe all pm_v calls should redirect to stderr.
Comment 2 Walter Schwarzenfeld freebsd_triage 2018-02-12 10:53:19 UTC
Maybe, related to bug #225840.
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-02-12 13:34:52 UTC
A commit references this bug:

Author: se
Date: Mon Feb 12 13:34:21 UTC 2018
New revision: 461586
URL: https://svnweb.freebsd.org/changeset/ports/461586

Log:
  Fix problem with debug output from export-flavor garbling the gen_dep_list
  result. The solution is different from the patch suggested in the PR, since
  the debug message was only meant to be used during early development of
  flavors support. In my local development version of portmaster, all output
  from pm_v is redirected to STDERR, but that change seemed to disruptive to
  apply to the "published" version.

  Unrelated changes: make pm_isdir_pd more robust and fix variable name of
  BACKUP_FORMAT option in sample file.

  PR:             225843
  Reported by:    John Hein
  Approved by:    antoine (implicit)

Changes:
  head/ports-mgmt/portmaster/Makefile
  head/ports-mgmt/portmaster/files/patch-files_portmaster.rc.sample
  head/ports-mgmt/portmaster/files/patch-portmaster
Comment 4 Stefan Eßer freebsd_committer freebsd_triage 2018-02-12 13:43:14 UTC
Thank you for reporting this issue. I did not test the "official" portmaster version with "-v", recently, and therefore did not notice it.

My development version on the way to portmaster-4.x has been sending all pm_v output to STDERR for quite some time, but that might break scripts that wrap portmaster and expect those messages output on STDOUT. I did not want to redirect the output from within one specific function, but the pm_v within export_flavors was only meant to help me trace flavor support early on, and I have now deleted it.

PR 225840 is unrelated. It is caused by pkg detecting that a file is installed by multiple flavors of a port, causing a conflict. This issue must be fixed in the py-trezor port and is not affected by portmaster at all.
Comment 5 John Hein 2018-02-13 17:46:19 UTC
Thanks for committing a fix.

I think any script that depends on portmaster -v output on stdout would be quite rare and is fragile anyway.  I think it's reasonable to redirect verbose information messages to stderr, but I'll leave it up to you.

p.s. I know the py-trezor was unrelated.  I just picked it as an example of a way to reproduce this portmaster problem.