bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnulib-tool.py: follow gnulib-tool changes


From: Dmitry Selyutin
Subject: Re: gnulib-tool.py: follow gnulib-tool changes
Date: Sat, 9 Sep 2017 19:04:13 +0300

> Well, the logic that binary files (*.mo, *.class) should be copied as-is,
> not transformed, should be kept, no? You'll replace the implementation
> of the transform?
The binary files shall not be touched, but text files shall not be processed
with sed. That's what I mean; the patch is OK, but the code around is flawed.

> If you replace 'sed' here, you save a subprocess
> invocation, though.
Exactly; I think one of the strongest motivations to perform the whole rewrite
in Python is the fact that the original gnulib-tool spawns processes too often.
Moreover, process invocation is quite a dumb technique when you have built-in
language features instead.

2017-09-09 18:41 GMT+03:00 Bruno Haible <address@hidden>:
Hi Dmitry,

> > [PATCH 6/6] gnulib-tool.py: follow gnulib-tool changes, part 14
> > gnulib-tool: don't transform binary files with sed
> All these sed transformers shall be IMHO entirely deprecated. I don't quite
> remember why I used sed

Using 'sed' is acceptable here because the input comes from a file and the
output goes to a file anyway. If you replace 'sed' here, you save a subprocess
invocation, though. This will be interesting when you/we are going to start
optimizing the thing.

Another possible optimization here is that first, we do a
  cp lookedup tmpfile
and then
  sed -e transformer < lookedup > tmpfile
We could eliminate the cp command when there is a transformer.

> be aware though that this part of code is going to be removed.

Well, the logic that binary files (*.mo, *.class) should be copied as-is,
not transformed, should be kept, no? You'll replace the implementation
of the transform?

> > [PATCH 5/6] gnulib-tool.py: follow gnulib-tool changes, part 13
> > gnulib-tool: concatenate lib_SOURCES to a single line
> A bit tricky one, but OK from my side. The only thing I noted is that
> `startpos,pos = match.span()` can be a bit better formatted into
> `(startpos, pos) = match.span()`.

Done. I had verified that both syntaxes work, but did not know which one is
the preferred one.

Bruno




--
With best regards,
Dmitry Selyutin

reply via email to

[Prev in Thread] Current Thread [Next in Thread]