linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yordan Karadzhov <y.karadz@gmail.com>
To: Solomon Tan <wjsota@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 1/2] kernel-shark: Make FreeSans a compulsory component
Date: Thu, 21 Apr 2022 15:04:50 +0300	[thread overview]
Message-ID: <04db0694-bb3b-3c9d-d2f0-09300f13a637@gmail.com> (raw)
In-Reply-To: <20220420153827.637413-1-wjsota@gmail.com>



On 20.04.22 г. 18:38 ч., Solomon Tan wrote:
> This patch is proposed for two reasons.
> 
> Firstly, with reference to
> https://lore.kernel.org/linux-trace-devel/Yl9WBztqKGD0PpmJ@ArchDesktop/
> when the CMake does not detect FreeSans, it should cease generating the
> Makefile since kernelshark requires FreeSans to output its graph
> properly.
> 
> Secondly, when the make instructions on the README are followed, even
> when FreeSans is not detected, `make` continues to compile and `make
> install` (or `install_gui`) would install without indicating an error.
> This gives the impression of a successful install, but `kernelshark`
> will be missing.
> 
> Therefore, making CMake quit with error when FreeSans is not installed
> solves these two issues.
> 
> Signed-off-by: Solomon Tan <wjsota@gmail.com>
> ---
>   CMakeLists.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index e6a76d8..5473bfa 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -69,7 +69,7 @@ if (TT_FONT_FILE)
>   
>   else (TT_FONT_FILE)
>   
> -  message(WARNING "\nCould not find font ${KS_FONT}!\n")
> +  message(FATAL_ERROR "\nCould not find font ${KS_FONT}!\n")

Hi Solomon,

Having this message as warning instead of a fatal error is done on purpose. We have other projects, that are using the 
kernelshark library (libkshark) but do not need the kernelshark GUI. So the library have to be able to build even if the 
third party packages needed by the GUI are not installed.

On the other hand I understand your argument that this can be confusing for someone who wants to build the GUI and do 
not care about the library. My suggestion is to keep the warning, but to make the message more informative, so that the 
user will be aware that the GUI is not going build.

And please send your patch-sets as new email threads.

Thanks!
Yordan

>   
>   endif (TT_FONT_FILE)
>   

      parent reply	other threads:[~2022-04-21 12:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-09 10:48 Kernelshark v2 all-black plots area Dario Faggioli
2021-02-09 13:06 ` Yordan Karadzhov (VMware)
2021-02-09 16:13   ` Dario Faggioli
2021-02-10 11:21     ` Yordan Karadzhov (VMware)
2021-02-10 14:51       ` Dario Faggioli
2021-02-10 15:47         ` Yordan Karadzhov (VMware)
2021-02-10 16:15           ` Dario Faggioli
2021-02-10 16:23           ` Steven Rostedt
2022-04-20  0:38   ` Solomon Tan
2022-04-20  8:06     ` Yordan Karadzhov
2022-04-20 15:17       ` Solomon Tan
2022-04-20 15:38         ` [PATCH 1/2] kernel-shark: Make FreeSans a compulsory component Solomon Tan
2022-04-20 15:38           ` [PATCH 2/2] kernel-shark: Allow FreeSans otf in addition to ttf Solomon Tan
2022-04-21 12:16             ` Yordan Karadzhov
2022-04-21 12:04           ` Yordan Karadzhov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=04db0694-bb3b-3c9d-d2f0-09300f13a637@gmail.com \
    --to=y.karadz@gmail.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=wjsota@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).