[PATCH] gnulib-tool.py: Append, don't replace existing VCS ignore files

[Darshit Shah]

* pygnulib/GLImport.py(_update_ignorelist_): Append the ignore data to
any existing VCS ignore files instead of replacing them
---
pygnulib/GLImport.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 4352a7ec5..b0e3ec655 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -726,9 +726,9 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
if not self.config['dryrun']:
print('Updating %s (backup in %s)' %
(srcpath, backupname))
- shutil.move(srcpath, backupname)
+ shutil.copy2(srcpath, backupname)
result = string()
- with codecs.open(srcpath, 'wb', 'UTF-8') as file:
+ with codecs.open(srcpath, 'a', 'UTF-8') as file:
file.write(destdata)
else: # if self.config['dryrun']
print('Updating %s (backup in %s)' %

[Eric Blake]

Post by Darshit Shah
* pygnulib/GLImport.py(_update_ignorelist_): Append the ignore data to
any existing VCS ignore files instead of replacing them
---
pygnulib/GLImport.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 4352a7ec5..b0e3ec655 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -726,9 +726,9 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
print('Updating %s (backup in %s)' %
(srcpath, backupname))
- shutil.move(srcpath, backupname)
+ shutil.copy2(srcpath, backupname)
result = string()

Shouldn't you use 'ab' to force the append in binary rather than text
mode, since the old code was using 'wb'?

[Darshit Shah]

Post by Eric Blake

Post by Darshit Shah
* pygnulib/GLImport.py(_update_ignorelist_): Append the ignore data to
any existing VCS ignore files instead of replacing them
---
pygnulib/GLImport.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 4352a7ec5..b0e3ec655 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -726,9 +726,9 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
print('Updating %s (backup in %s)' %
(srcpath, backupname))
- shutil.move(srcpath, backupname)
+ shutil.copy2(srcpath, backupname)
result = string()

Shouldn't you use 'ab' to force the append in binary rather than text
mode, since the old code was using 'wb'?
Note
Underlying encoded files are always opened in binary mode. No automatic
conversion of '\n' is done on reading and writing. The mode argument may
be any binary mode acceptable to the built-in open() function; the 'b'
is automatically added.

So, the 'b' is not strictly required. However, since you mention it, it
may be sensible to add it nonetheless just to be more explicit about the
expected behavior.

[1]: https://docs.python.org/3/library/codecs.html

[Bruno Haible]

Hi Darshit,

Post by Darshit Shah
* pygnulib/GLImport.py(_update_ignorelist_): Append the ignore data to
any existing VCS ignore files instead of replacing them

We cannot judge this patch, for lack of information about what it does and
why it is needed.

1) What is the problem with the current behaviour of gnulib-tool.py?

2) Is the behaviour of gnulib-tool the same? That is, is this patch introducing
a behavioural difference between gnulib-tool.py and gnulib-tool, or is it
removing one?

Some background: We plan to have, for some time, gnulib-tool and gnulib-tool.py
be functionally identical, i.e. they should produce the exactly same results
and outputs. During this time,
- Dmitry and others will be able to rewrite or optimize gnulib-tool.py,
- Anyone can compare the results and speed of gnulib-tool vs. gnulib-tool.py.

But we are not there yet. Maybe in one or two weeks, but not now. Until then,
it is essential that gnulib-tool.py *converges* to gnulib-tool. We want to
avoid diverging behaviour.

And when we are there, it will be essential that gnulib-tool.py and gnulib-tool
evolve in sync. That is, any new feature of gnulib-tool should be a new
feature of gnulib-tool.py, and vice versa.

Bruno

[Darshit Shah]

Hi Bruno,

Post by Bruno Haible
Hi Darshit,

Post by Darshit Shah
* pygnulib/GLImport.py(_update_ignorelist_): Append the ignore data to
any existing VCS ignore files instead of replacing them

We cannot judge this patch, for lack of information about what it does and
why it is needed.

Sorry, I meant to have a detailed description sent along with the patch,
but I guess I forgot to add the cover-letter in my command. Hence, let
me explain it here

Post by Bruno Haible
1) What is the problem with the current behaviour of gnulib-tool.py?

With the existing behaviour, gnulib-tool.py will replace the old VCS
ignore file (.gitignore in the case of Wget2) with a list it created
internally. Hence, on a fresh checkout of Wget2 which now uses
gnulib-tool.py as the gnulib driver, when one executes "./bootstrap",
the existing `.gitignore` file in the project is replaced with one
created by gnulib-tool.py, removing all the ignore rules set by the
project for that repository.

On top of it, there is another bug which I have not yet managed to nail
down. That is, it tries to update "tests/.gitignore" twice. First, with
a list of new files that it has created, and then again with an empty
list. With the existing behaviour, it creates a new "tests/.gitignore"
file and then replaces it with an empty file as a result.

My patch fixes both these cases, however it does not fix the bug where
"tests/.gitignore" is created and then updated in the same run. Neither
does it fix the issue I raised in a different email to this list about
gnulib-tool.py creating new files in the "tests/" directory.

Post by Bruno Haible
2) Is the behaviour of gnulib-tool the same? That is, is this patch introducing
a behavioural difference between gnulib-tool.py and gnulib-tool, or is it
removing one?

As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Post by Bruno Haible
Some background: We plan to have, for some time, gnulib-tool and gnulib-tool.py
be functionally identical, i.e. they should produce the exactly same results
and outputs. During this time,
- Dmitry and others will be able to rewrite or optimize gnulib-tool.py,
- Anyone can compare the results and speed of gnulib-tool vs. gnulib-tool.py.
But we are not there yet. Maybe in one or two weeks, but not now. Until then,
it is essential that gnulib-tool.py *converges* to gnulib-tool. We want to
avoid diverging behaviour.
And when we are there, it will be essential that gnulib-tool.py and gnulib-tool
evolve in sync. That is, any new feature of gnulib-tool should be a new
feature of gnulib-tool.py, and vice versa.

I understand this very well. I've been following the mailing lists and
the development of gnulib-tool.py. Dmitry's work with the Python script
has been great and this definitely helps in reducing the time it takes
to run bootstrap on a fresh checkout.

[Dmitry Selyutin]

Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you push it
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best, though my
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is working
somehow and can be a basic for a much better API. Remember that we're staying
on giants' shoulders, I've just rewritten what was created before in another
language. :-) Anyway, thank you for your kind words!

[Darshit Shah]

Post by Dmitry Selyutin
Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you push it
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best, though my
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is working
somehow and can be a basic for a much better API. Remember that we're staying
on giants' shoulders, I've just rewritten what was created before in another
language. :-) Anyway, thank you for your kind words!
--
With best regards,
Dmitry Selyutin

Bruno,

Could you please push this patch to master?

[Darshit Shah]

Hi,

Is there something blocking this patch? It doesn't seem like it has been merged
yet.

Post by Darshit Shah

Post by Dmitry Selyutin
Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you push it
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best, though my
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is working
somehow and can be a basic for a much better API. Remember that we're staying
on giants' shoulders, I've just rewritten what was created before in another
language. :-) Anyway, thank you for your kind words!
--
With best regards,
Dmitry Selyutin

Bruno,
Could you please push this patch to master?
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

[Darshit Shah]

It's been a while. Atleast some response on why it isn't being accepted would
be appreciated

Post by Darshit Shah
Hi,
Is there something blocking this patch? It doesn't seem like it has been merged
yet.

Post by Darshit Shah

Post by Dmitry Selyutin
Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you push it
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best, though my
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is working
somehow and can be a basic for a much better API. Remember that we're staying
on giants' shoulders, I've just rewritten what was created before in another
language. :-) Anyway, thank you for your kind words!
--
With best regards,
Dmitry Selyutin

Bruno,
Could you please push this patch to master?
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

[Dmitry Selyutin]

Hi Darshit,

the reason is actually very simple: I just had no time to apply it due to
activities on a regular work. Hopefully I'll continue developing
gnulib-tool in the end of this week. The patch seems to be OK though; I'll
try to push it this evening. Thank you again and sorry for the delay.

20 МПяб. 2017 г. 2:53 ДП пПльзПватель "Darshit Shah" <***@gmail.com>
МапОсал:

It's been a while. Atleast some response on why it isn't being accepted
would
be appreciated

Post by Darshit Shah
Hi,
Is there something blocking this patch? It doesn't seem like it has been merged
yet.

Post by Darshit Shah

Post by Dmitry Selyutin
Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also consider to
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you push it
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best, though my
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is working
somehow and can be a basic for a much better API. Remember that we're staying
on giants' shoulders, I've just rewritten what was created before in another
language. :-) Anyway, thank you for your kind words!
--
With best regards,
Dmitry Selyutin

Bruno,
Could you please push this patch to master?
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

[Darshit Shah]

Hi Dmitry,

That's perfectly fine. Long as I know that it is being looked into, it's okay.

There's always so many things that are going on and other items often take
priority. No issues.

Post by Bruno Haible
Hi Darshit,
the reason is actually very simple: I just had no time to apply it due to
activities on a regular work. Hopefully I'll continue developing
gnulib-tool in the end of this week. The patch seems to be OK though; I'll
try to push it this evening. Thank you again and sorry for the delay.
It's been a while. Atleast some response on why it isn't being accepted would
be appreciated

Post by Darshit Shah
Hi,
Is there something blocking this patch? It doesn't seem like it has been

merged

Post by Darshit Shah
yet.

Post by Darshit Shah

Post by Dmitry Selyutin
Hi all,

Post by Darshit Shah
As explained above, this patch helps converge gnulib-tool.py and
gnulib-tool. There is a behavioural difference which I also

consider to

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin

Post by Darshit Shah
be a regression, and hence a string need to fix it.

Thanks for the patch! Looks perfectly fine from my side. Could you

push it

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin
into the repository, please?

Post by Darshit Shah
But we are not there yet. Maybe in one or two weeks, but not now.

I think it is a way too optimistic forecast. Anyway, I do my best,

though my

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin
Python skills are a little bit rusty. :-)

Post by Darshit Shah
Dmitry's work with the Python script has been great

Well, I wouldn't say after looking into the code, but at least it is

working

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin
somehow and can be a basic for a much better API. Remember that we're

staying

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin
on giants' shoulders, I've just rewritten what was created before in

another

Post by Darshit Shah

Post by Darshit Shah

Post by Dmitry Selyutin
language. :-) Anyway, thank you for your kind words!
--
With best regards,
Dmitry Selyutin

Bruno,
Could you please push this patch to master?
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

[Dmitry Selyutin]

Hi Darshit,

I've just pushed your patch. I took the liberty to
format it as if it was sent via git format-patch.
Thank you again!

[Darshit Shah]

Thanks!

Post by Bruno Haible
Hi Darshit,
I've just pushed your patch. I took the liberty to
format it as if it was sent via git format-patch.
Thank you again!