public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: Markus Heiser <markus.heiser@darmarit.de>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kees Cook <keescook@chromium.org>,
	Anton Vorontsov <anton@enomsg.org>,
	Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH v2 00/11] Documentation: Add ABI to the admin guide
Date: Fri, 21 Apr 2017 11:22:21 -0300	[thread overview]
Message-ID: <20170421112221.58d1379b@vento.lan> (raw)
In-Reply-To: <9A271A6C-3B84-4F27-A852-CCA7AD404AB0@darmarit.de>

Em Fri, 21 Apr 2017 08:37:45 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 21.04.2017 um 01:21 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> 
> > - I'm not a python programmer ;-) I just took Markus "generic" kernel-cmd
> >  code, hardcoding there a call to the script.
> > 
> >  With (a lot of) time, I would likely be able to find a solution to add
> >  the entire ABI logic there, but, in this case, we would lose the
> >  capability of calling the script without Sphinx.  
> 
> Hi Mauro,
> 
> I have no problem with calling the perl script, but your last sentence
> is not correct: We can implement a python module, which is used by sphinx
> and we can add a command line as well.

Markus,

Yeah, I guess technically it would be possible to make a Sphinx plugin 
that would also allow have a command line interface. I don't like this
kind of option, because the code can be come messy.

A better design would be to create a library and make two interfaces
for it, one for the ReST plugin and another one to be called via a
command line, but, in this case, we'll need to maintain 3 interdependent
components (library, command line, Sphinx plugin) instead of two (almost)
independent ones (a script and a Sphinx plugin).

So, IMO, the design I took is a good one, and it has a big advantage:
writing in perl is a way easier for me, and I can benefit from my
knowledge to write a script that performs well. On my desktop, it can
parse the entire ABI, search for a string there and output its result
in 100ms:

$ time ./scripts/get_abi.pl search usbip_status

/sys/devices/platform/usbip-vudc.%d/usbip_status
------------------------------------------------

Kernel version:		4.6
Date:			April 2016
Contact:		Krzysztof Opasiak <k.opasiak@samsung.com>
Defined on file:	Documentation/ABI/testing/sysfs-platform-usbip-vudc

Description:

Current status of the device.
Allowed values:
1 - Device is available and can be exported
2 - Device is currently exported
3 - Fatal error occurred during communication
  with peer


real	0m0.112s
user	0m0.106s
sys	0m0.005s

---

With regards to the decision of using perl instead of python,
see below. One might thing it is rant. It isn't. It is just a
matter of optimizing my development time.

I have lots of bad experiences with patchwork and even with
cgit (running on an old LTS debian machine) due to the lack of 
a consistent backward compatible API on python.

Most of my bad experiences on python scripts is related to how badly
it handles a file input with an unknown charset and unsigned chars > 0x7f. 
If Python can't recognize the a char as a valid ascii character 
(or the charset was not explicity defined - or the script doesn't
 know what's the original encoding charset),  the script crashes 
(ok, one could add a "try" block, but it is very painful to do that
all over the code). Also, the way the charset is specified on a python 
script changed several times during 2.x development (causing incompatible
changes), and again on 3.x. It is even worse if a python script is called 
by some other script, as, in such case, Python (not sure if all versions)
ignores script headers like:
	# -*- coding: utf-8 -*-

with would otherwise tell what's the default encoding.

If you look at patchwork git tree, you'll see that it took a really long
time since when the first patch trying to address those issues until the
last one was merged.

That's the first patch there[1]:

	commit ea39a9952e3fa647ebcb4bf16981ce941ec5236a
	Author: Mauro Carvalho Chehab <mchehab@infradead.org>
	Date:   Tue Nov 18 23:00:32 2008 -0200

	    Fix non-ascii character encodings on xmlrpc interface

That seems to be the last one[2]:
	commit 880fc52d2d4ccdcbf4a7b76f1b4ba6b9e7482dff
	Author: Siddhesh Poyarekar <siddhesh@redhat.com>
	Date:   Mon Jul 14 10:21:32 2014 +0800

	    parsemail: Fallback to common charsets when charset is None or x-unknown

AFAIKT, none of those charset fixes are due to a code regression, but,
instead, fixing parsing on different places of the code. So, it took
at least 6 years to get it right there.

So, my decision of writing it in perl is basically due to the fact
that I can write a reliable script it in a few hours, and won't 
need to be concerned that some weird char inside a file or some
new scripting interpreter version would cause my script to crash.

[1] git://github.com/getpatchwork/patchwork
[2] I guess I hit other patchwork charset bugs after 2014.

Thanks,
Mauro

      reply	other threads:[~2017-04-21 14:22 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-13 10:08 [PATCH v2 00/11] Documentation: Add ABI to the admin guide Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 01/11] doc-rst: customize RTD theme; literal-block Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 02/11] ABI: fix some syntax issues at the ABI database Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 03/11] ABI: sysfs-driver-hid: the "What" field doesn't parse fine Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 04/11] ABI: sysfs-class-uwb_rc: remove a duplicated incomplete entry Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 05/11] ABI: better identificate tables Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 06/11] scripts: add an script to parse the ABI files Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 07/11] scripts/get_abi.pl: parse files with text at beginning Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 08/11] scripts/get_abi.pl: avoid use literal blocks when not needed Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 09/11] scripts/get_abi.pl: split label naming from xref logic Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 10/11] scripts/get_abi.pl: add support for searching for ABI symbols Mauro Carvalho Chehab
2017-04-13 10:08 ` [PATCH v2 11/11] doc-rst: add ABI documentation to the admin-guide book Mauro Carvalho Chehab
2017-04-13 11:02 ` [PATCH v2 00/11] Documentation: Add ABI to the admin guide Mauro Carvalho Chehab
     [not found] ` <20170413102612.BA6BCC603E@b03ledav006.gho.boulder.ibm.com>
2017-04-13 14:02   ` [PATCH v2 02/11] ABI: fix some syntax issues at the ABI database Andrew Donnellan
2017-04-20 21:40 ` [PATCH v2 00/11] Documentation: Add ABI to the admin guide Jonathan Corbet
2017-04-20 23:21   ` Mauro Carvalho Chehab
2017-04-21  6:37     ` Markus Heiser
2017-04-21 14:22       ` Mauro Carvalho Chehab [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=20170421112221.58d1379b@vento.lan \
    --to=mchehab@s-opensource.com \
    --cc=anton@enomsg.org \
    --cc=ccross@android.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus.heiser@darmarit.de \
    --cc=mchehab@infradead.org \
    --cc=tony.luck@intel.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