All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.