| Message ID | 1479294405-17471-1-git-send-email-rf@opensource.wolfsonmicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 16, 2016 at 11:06:45AM +0000, Richard Fitzgerald wrote: > For some reason when the build system was converted to > automake by this patch > > "Convert the build system to autotools" > > the name of the output binary for crec was changed > from 'crec' to 'crecord'. > > This patch corrects it back to 'crec' That implies crecord is incorrect, I do not think so. Can you explain why you would want to rename this back. The motivation for this was to make it proper like cplay. Also similar to aplay and arecord.
On Wed, 2016-11-16 at 18:32 +0530, Vinod Koul wrote: > On Wed, Nov 16, 2016 at 11:06:45AM +0000, Richard Fitzgerald wrote: > > For some reason when the build system was converted to > > automake by this patch > > > > "Convert the build system to autotools" > > > > the name of the output binary for crec was changed > > from 'crec' to 'crecord'. > > > > This patch corrects it back to 'crec' > > That implies crecord is incorrect, I do not think so. > > Can you explain why you would want to rename this back. > > The motivation for this was to make it proper like cplay. Also similar to > aplay and arecord. > I don't really see a need for that, or why "crec" is wrong. In any case if you did want to change that name a) it should be an explicit patch, not sneaked in as an undocumented and unexpected side-effect of some other patch b) the source file should have been renamed to match, if the name "crec" is a massive problem then it should also be a problem that "cplay.c->cplay" but "crec.c->crecord". If it's ok to have the source file called crec.c what's wrong with the binary it builds being called crec? c) it's a nuisance to have to change existing test systems and apps that launch "crec", or to create symlinks from crec->crecord.
On Wed, Nov 16, 2016 at 01:14:21PM +0000, Richard Fitzgerald wrote: > On Wed, 2016-11-16 at 18:32 +0530, Vinod Koul wrote: > > On Wed, Nov 16, 2016 at 11:06:45AM +0000, Richard Fitzgerald wrote: > > > For some reason when the build system was converted to > > > automake by this patch > > > > > > "Convert the build system to autotools" > > > > > > the name of the output binary for crec was changed > > > from 'crec' to 'crecord'. > > > > > > This patch corrects it back to 'crec' > > > > That implies crecord is incorrect, I do not think so. > > > > Can you explain why you would want to rename this back. > > > > The motivation for this was to make it proper like cplay. Also similar to > > aplay and arecord. > > > > I don't really see a need for that, or why "crec" is wrong. > > In any case if you did want to change that name > > a) it should be an explicit patch, not sneaked in as an undocumented and > unexpected side-effect of some other patch This part I agree. Unfortunately this was not done transparently... > b) the source file should have been renamed to match, if the name "crec" > is a massive problem then it should also be a problem that > "cplay.c->cplay" but "crec.c->crecord". If it's ok to have the source > file called crec.c what's wrong with the binary it builds being called > crec? I dont mind renaming source as well.. I think bigger question is should we continue crec or crecord, my preference is latter. > c) it's a nuisance to have to change existing test systems and apps that > launch "crec", or to create symlinks from crec->crecord. Yeah agree, lets decide on one and stick to that, forever hopefully :)
On Fri, 2016-11-18 at 09:26 +0530, Vinod Koul wrote: > On Wed, Nov 16, 2016 at 01:14:21PM +0000, Richard Fitzgerald wrote: > > On Wed, 2016-11-16 at 18:32 +0530, Vinod Koul wrote: > > > On Wed, Nov 16, 2016 at 11:06:45AM +0000, Richard Fitzgerald wrote: > > > > For some reason when the build system was converted to > > > > automake by this patch > > > > > > > > "Convert the build system to autotools" > > > > > > > > the name of the output binary for crec was changed > > > > from 'crec' to 'crecord'. > > > > > > > > This patch corrects it back to 'crec' > > > > > > That implies crecord is incorrect, I do not think so. > > > > > > Can you explain why you would want to rename this back. > > > > > > The motivation for this was to make it proper like cplay. Also similar to > > > aplay and arecord. > > > > > > > I don't really see a need for that, or why "crec" is wrong. > > > > In any case if you did want to change that name > > > > a) it should be an explicit patch, not sneaked in as an undocumented and > > unexpected side-effect of some other patch > > This part I agree. Unfortunately this was not done transparently... > > > b) the source file should have been renamed to match, if the name "crec" > > is a massive problem then it should also be a problem that > > "cplay.c->cplay" but "crec.c->crecord". If it's ok to have the source > > file called crec.c what's wrong with the binary it builds being called > > crec? > > I dont mind renaming source as well.. > > I think bigger question is should we continue crec or crecord, my preference > is latter. > > > c) it's a nuisance to have to change existing test systems and apps that > > launch "crec", or to create symlinks from crec->crecord. > > Yeah agree, lets decide on one and stick to that, forever hopefully :) > Ok let's change to crecord for consistency with arecord, and I'll do a patch to rename the source to match.
On Fri, Nov 18, 2016 at 10:13:10AM +0000, Richard Fitzgerald wrote: > > This part I agree. Unfortunately this was not done transparently... > > > > > b) the source file should have been renamed to match, if the name "crec" > > > is a massive problem then it should also be a problem that > > > "cplay.c->cplay" but "crec.c->crecord". If it's ok to have the source > > > file called crec.c what's wrong with the binary it builds being called > > > crec? > > > > I dont mind renaming source as well.. > > > > I think bigger question is should we continue crec or crecord, my preference > > is latter. > > > > > c) it's a nuisance to have to change existing test systems and apps that > > > launch "crec", or to create symlinks from crec->crecord. > > > > Yeah agree, lets decide on one and stick to that, forever hopefully :) > > > > Ok let's change to crecord for consistency with arecord, and I'll do a > patch to rename the source to match. Sounds good :)
diff --git a/src/utils/Makefile.am b/src/utils/Makefile.am index 5f685e3..1967d92 100644 --- a/src/utils/Makefile.am +++ b/src/utils/Makefile.am @@ -1,11 +1,11 @@ -bin_PROGRAMS = cplay crecord +bin_PROGRAMS = cplay crec cplay_SOURCES = cplay.c -crecord_SOURCES = crec.c +crec_SOURCES = crec.c cplay_CFLAGS = -I$(top_srcdir)/include -crecord_CFLAGS = -I$(top_srcdir)/include +crec_CFLAGS = -I$(top_srcdir)/include cplay_LDADD = $(top_builddir)/src/lib/libtinycompress.la -crecord_LDADD = $(top_builddir)/src/lib/libtinycompress.la +crec_LDADD = $(top_builddir)/src/lib/libtinycompress.la
For some reason when the build system was converted to automake by this patch "Convert the build system to autotools" the name of the output binary for crec was changed from 'crec' to 'crecord'. This patch corrects it back to 'crec' Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com> --- src/utils/Makefile.am | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)