Discussion:
Not distributing README-release automatically.
(too old to reply)
Mathieu Lirzin
2017-03-05 16:12:27 UTC
Permalink
Raw Message
Hello,

I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.

After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
bootstrap that could be removed:

--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---

IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).

What do people think?
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Jim Meyering
2017-03-07 05:45:27 UTC
Permalink
Raw Message
Post by Mathieu Lirzin
Hello,
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Hi Mathieu,
Thanks for helping with automake.

Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
Mathieu Lirzin
2017-03-07 16:08:55 UTC
Permalink
Raw Message
Hello Jim,
Post by Jim Meyering
Post by Mathieu Lirzin
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
That would be fine with me. However I am not sure this additional logic
would be smaller (or simpler), since IIUC there is currently only 3
top-file-specifying modules:

- maintainer-makefile
- readme-release
- gnumakefile

Do I overlook something?

Thanks.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Jim Meyering
2017-03-07 16:12:59 UTC
Permalink
Raw Message
Post by Mathieu Lirzin
Hello Jim,
Post by Jim Meyering
Post by Mathieu Lirzin
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
That would be fine with me. However I am not sure this additional logic
would be smaller (or simpler), since IIUC there is currently only 3
- maintainer-makefile
- readme-release
- gnumakefile
Do I overlook something?
I hadn't counted, and expected more. Given there are only three,
adding an 'EXTRA_DIST += ...' in two of the three seems reasonable. Do
you feel like writing the patch?
Mathieu Lirzin
2017-03-07 17:50:05 UTC
Permalink
Raw Message
Post by Jim Meyering
Post by Mathieu Lirzin
Hello Jim,
Post by Jim Meyering
Post by Mathieu Lirzin
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
That would be fine with me. However I am not sure this additional logic
would be smaller (or simpler), since IIUC there is currently only 3
- maintainer-makefile
- readme-release
- gnumakefile
Do I overlook something?
I hadn't counted, and expected more. Given there are only three,
adding an 'EXTRA_DIST += ...' in two of the three seems reasonable. Do
you feel like writing the patch?
Yes, I will do that.

Thank you.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Mathieu Lirzin
2017-03-14 11:34:34 UTC
Permalink
Raw Message
Post by Jim Meyering
Post by Mathieu Lirzin
Post by Jim Meyering
Post by Mathieu Lirzin
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
That would be fine with me. However I am not sure this additional logic
would be smaller (or simpler), since IIUC there is currently only 3
- maintainer-makefile
- readme-release
- gnumakefile
Do I overlook something?
I hadn't counted, and expected more. Given there are only three,
adding an 'EXTRA_DIST += ...' in two of the three seems reasonable. Do
you feel like writing the patch?
Here is the patch:
Jim Meyering
2017-03-14 17:02:09 UTC
Permalink
Raw Message
Post by Jim Meyering
Post by Mathieu Lirzin
Post by Jim Meyering
Post by Mathieu Lirzin
I have been using 'readme-release' module and was surprised to discover
that the "README-release" file was automatically distributed without any
mention in the "module/readme-release" file.
After some digging I have discovered that all files from the "top"
directory are automatically added to EXTRA_DIST. Here is a snippet from
--8<---------------cut here---------------start------------->8---
if test -n "$top_files"; then
sed_prepend_topdir='s,^,$(top_srcdir)/,'
echo "EXTRA_DIST += "`echo "$top_files" | sed -e "$sed_prepend_topdir"`
echo
fi
--8<---------------cut here---------------end--------------->8---
IMO we should move the responsability of distributing "top/*" files to
the module definition instead, and not distributing "README-release"
automatically (like what is done in Coreutils).
What do people think?
Regarding README-release, I'm leery of requiring each
top-file-specifying module writer to remember to add each such file to
EXTRA_DIST. An alternative, probably-smaller change may be to add some
attribute by which a module with such a file could opt out of that
default.
That would be fine with me. However I am not sure this additional logic
would be smaller (or simpler), since IIUC there is currently only 3
- maintainer-makefile
- readme-release
- gnumakefile
Do I overlook something?
I hadn't counted, and expected more. Given there are only three,
adding an 'EXTRA_DIST += ...' in two of the three seems reasonable. Do
you feel like writing the patch?
At first glance, that diff looks perfect.
The only missing piece (actually important, since not easily derived
by looking at the patch) is justification: why make this change. Would
you please adjust the log/ChangeLog to say it is to stop distributing
README-release?
Bruno Haible
2017-03-14 17:34:14 UTC
Permalink
Raw Message
+EXTRA_DIST += top/GNUmakefile
This should read

EXTRA_DIST += $(top_srcdir)/GNUmakefile
+EXTRA_DIST += top/maint.mk
This should read

EXTRA_DIST += $(top_srcdir)/maint.mk

otherwise gnulib-tool's --create-testdir with the named modules will break.

Can you please revisit this (i.e. test --create-testdir once without and once
with my proposed correction)?

Bruno
Jim Meyering
2017-03-14 18:21:17 UTC
Permalink
Raw Message
Post by Bruno Haible
+EXTRA_DIST += top/GNUmakefile
This should read
EXTRA_DIST += $(top_srcdir)/GNUmakefile
+EXTRA_DIST += top/maint.mk
This should read
EXTRA_DIST += $(top_srcdir)/maint.mk
otherwise gnulib-tool's --create-testdir with the named modules will break.
Can you please revisit this (i.e. test --create-testdir once without and once
with my proposed correction)?
Thank you, Bruno. Good catch.
Mathieu Lirzin
2017-03-14 18:28:03 UTC
Permalink
Raw Message
Hi,
Post by Bruno Haible
+EXTRA_DIST += top/GNUmakefile
This should read
EXTRA_DIST += $(top_srcdir)/GNUmakefile
+EXTRA_DIST += top/maint.mk
This should read
EXTRA_DIST += $(top_srcdir)/maint.mk
otherwise gnulib-tool's --create-testdir with the named modules will break.
Can you please revisit this (i.e. test --create-testdir once without and once
with my proposed correction)?
Bruno
I didn't know how to test my patch without setting up a project
manually. Thank you for explaining me how to do that. I was able to
identify my mistake then.

Here is an updated patch with a rationale in the ChangeLog:
Jim Meyering
2017-03-14 18:42:24 UTC
Permalink
Raw Message
On Tue, Mar 14, 2017 at 11:28 AM, Mathieu Lirzin <***@gnu.org> wrote:
...
Thanks for the review (to both of you).
Thank you. I've pushed that.

Loading...