[emms-help] 4.2 release tasks: emms-print-metadata install

Post by Yoni Rabkin
In 4.2 I want emms-print-metadata to be installed by `make install',
just like the manual says it is.

So just to remind of the general status. Petteri Hintsanen is working on
a TagLib C++ version of the emms-print-metadata. Also, Taglib was
recently updated to 1.11, which has better support for some tags like
album artist, and AFAIR the C++ interface offers better access to tags.
So that’s a good improvement.

Post by Yoni Rabkin
As far as I can tell emms-print-metadata isn't being installed into
/usr/bin/ by `make install'. Can people confirm this?

But that wouldn't work since we'd need root privilege to write to
/usr/bin/ and isn’t ELPA friendly.

I think the best way to do this would be to allow it to be compiled during
ELPA installation. pdf-mode does this, though emms-print-metadata is
probably a whole lot more simple.

Some of the pdf-mode built instructions:

https://github.com/politza/pdf-tools/blob/87690b7b568ae6145dfa4ffc9fc085b80ed039fa/lisp/pdf-tools.el#L249

Rasmus

--
Not everything that goes around comes back around, you know

Yoni Rabkin

2016-10-25 12:24:03 UTC

Thank you for the quick response.

Post by Yoni Rabkin
In 4.2 I want emms-print-metadata to be installed by `make install',
just like the manual says it is.

So just to remind of the general status. Petteri Hintsanen is working
on a TagLib C++ version of the emms-print-metadata. Also, Taglib was
recently updated to 1.11, which has better support for some tags like
album artist, and AFAIR the C++ interface offers better access to
tags. So that’s a good improvement.

OK, so there is a good reason to move to C++ with this, which would
otherwise seem like overkill to me.

Since we would want to test it a bit before packing it up into a release
it can hopefully go out with the release after this one (4.3, which is
scheduled for May 2017).

Post by Yoni Rabkin
As far as I can tell emms-print-metadata isn't being installed into
/usr/bin/ by `make install'. Can people confirm this?

But that wouldn't work since we'd need root privilege to write to
/usr/bin/ and isn’t ELPA friendly.

What we do with the standard install process shouldn't be influenced by
ELPA. Especially since Emms isn't in the ELPA at the moment (more on
this later).

We already have a `make install' process which requires root and is a
standard feature. So there is no additional cost to adding something
which installs to /usr/bin as well. In any case, `make all' doesn't
compile emms-print-metadata, so `sudo make install' wouldn't install
to /usr/bin unless there is an executable ready.

As such, I'll be adding installation of emms-print-metadata to the
Makefile for this release (4.2).

Post by Rasmus
I think the best way to do this would be to allow it to be compiled
during ELPA installation. pdf-mode does this, though
emms-print-metadata is probably a whole lot more simple.

Adding Emms to ELPA is a good thing, and should be a goal for the
release after this one (4.3).

Post by Rasmus
https://github.com/politza/pdf-tools/blob/87690b7b568ae6145dfa4ffc9fc085b80ed039fa/lisp/pdf-tools.el#L249
Rasmus

I'll file that url away for the next release.

Thanks!

--
"Cut your own wood and it will warm you twice"

Rasmus

2016-10-25 13:52:43 UTC

Post by Yoni Rabkin
OK, so there is a good reason to move to C++ with this, which would
otherwise seem like overkill to me.
Since we would want to test it a bit before packing it up into a release
it can hopefully go out with the release after this one (4.3, which is
scheduled for May 2017).

That should be fine. I don't know what the status is of this ATM.

Post by Yoni Rabkin
[...]
As such, I'll be adding installation of emms-print-metadata to the
Makefile for this release (4.2).

Seems fair. Thanks for fixing this.

Post by Yoni Rabkin
Adding Emms to ELPA is a good thing, and should be a goal for the
release after this one (4.3).

Sounds good! Thanks for that as well.

--
Hooray!

Petteri Hintsanen

2016-12-10 16:02:43 UTC

That should be fine. I don't know what the status is of this ATM.

Hello and apologies for not responding in a timely manner. For some
weird reason I had "dropped" EMMS folder in my own mail setup, so these
messages didn't catch me until now.

The patch is ok. It's a really simple piece of C++98 so it ought to
work with almost any compiler. I haven't submitted the patch earlier
because the docs were missing. But I have updated the texinfo and the
man page, so I'll post the patch Real Soon Now.

Some relevant copy-paste from correspondence with Rasmus last February:

"Btw, there is also a (feature-wise) limited Perl implementation in
emms/src. emms.texi does not mention anything about it. The program
uses Audio::Scan which seems to be yet another C-based parser for many
open media formats. I think we should either drop it or at least
document it."

regards,
Petteri

Yoni Rabkin

2017-02-19 16:40:10 UTC

Post by Petteri Hintsanen

That should be fine. I don't know what the status is of this ATM.

Hello and apologies for not responding in a timely manner. For some
weird reason I had "dropped" EMMS folder in my own mail setup, so
these messages didn't catch me until now.
The patch is ok. It's a really simple piece of C++98 so it ought to
work with almost any compiler. I haven't submitted the patch earlier
because the docs were missing. But I have updated the texinfo and the
man page, so I'll post the patch Real Soon Now.
"Btw, there is also a (feature-wise) limited Perl implementation in
emms/src. emms.texi does not mention anything about it. The program
uses Audio::Scan which seems to be yet another C-based parser for many
open media formats. I think we should either drop it or at least
document it."

Can I please have the latest and greatest version of your C++
emms-print-metadata patch to install?

I want people to be able to try it out well in advance of 4.3.

--
"Cut your own wood and it will warm you twice"

Alex Kost

2016-10-30 13:23:10 UTC

Post by Yoni Rabkin
In 4.2 I want emms-print-metadata to be installed by `make install',
just like the manual says it is.
As far as I can tell emms-print-metadata isn't being installed into
/usr/bin/ by `make install'. Can people confirm this?

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Post by Yoni Rabkin
Is there an important historical reason why this is the case or is it a
simple oversight?

--
Alex

Alex Kost

2016-10-30 13:26:45 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Oops, I've just noticed the other your message where you mentioned this,
so apparently this confirmation wasn't needed, sorry :-)

--
Alex

Yoni Rabkin

2016-10-30 13:27:21 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Thank you for this.

I've fixed this and it will be released along with version 4.2 in
November.

--
"Cut your own wood and it will warm you twice"

Alex Kost

2016-11-27 19:41:27 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Thank you for this.
I've fixed this and it will be released along with version 4.2 in
November.

I see that you didn't add 'emms-print-metadata' to 'all' target in
commit cbbe098¹, so "make" still doesn't build it. Was it intentional?

Also you use:

install -m 755 $(SRCDIR)/emms-print-metadata $(BINDIR)/emms-print-metadata;

This will fail if BINDIR does not exist (this is the case for such
systems as GuixSD or NixOS). Could you please also add "-D" flag to
"install" command?

¹ http://git.savannah.gnu.org/cgit/emms.git/commit/?id=cbbe0980565bc6e9b83d04e11d11f4c7aed9ff4f

--
Alex

Yoni Rabkin

2016-11-27 22:12:22 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Thank you for this.
I've fixed this and it will be released along with version 4.2 in
November.

I see that you didn't add 'emms-print-metadata' to 'all' target in
commit cbbe098¹, so "make" still doesn't build it. Was it intentional?

Yes. At this point we still want to keep the make phase to be about
elisp. This should be resolved when we decide how to do the compilation
in way compatible with remotely installable elisp packages.

Post by Alex Kost
install -m 755 $(SRCDIR)/emms-print-metadata $(BINDIR)/emms-print-metadata;
This will fail if BINDIR does not exist (this is the case for such
systems as GuixSD or NixOS). Could you please also add "-D" flag to
"install" command?

Do you have access to those systems? Can you do a test to see that it
would work as intended and send a patch?

--
"Cut your own wood and it will warm you twice"

Alex Kost

2016-12-04 15:12:00 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Thank you for this.
I've fixed this and it will be released along with version 4.2 in
November.

I see that you didn't add 'emms-print-metadata' to 'all' target in
commit cbbe098¹, so "make" still doesn't build it. Was it intentional?

Yes. At this point we still want to keep the make phase to be about
elisp. This should be resolved when we decide how to do the compilation
in way compatible with remotely installable elisp packages.

Sorry, I don't understand; do you mean installation from ELPA? But it
doesn't care about Makefile at all, so I don't see what problem can be
caused by adding 'emms-print-metadata' to 'all' target.

Do you have access to those systems? Can you do a test to see that it
would work as intended and send a patch?

Yes, I can confirm (I use GuixSD and I am one of the Guix contributors),
but after all it's not a big thing. It's one of the several (small)
issues with the EMMS Makefile that we make workarounds for in our emms
package: the same problem happens with "man" directory (Makefile also
assumes it exists), also info manual is put in "$(PREFIX)/info" instead
of "$(PREFIX)/share/info".

To recap, it is fixed on the Guix side, so you may ignore this message :-)

If you (or someone who is reading) are interested: GNU Guix¹ is a
package manager and GuixSD is a system thereof. It uses Guile
programming language, so people who like sexps may like it too :-)

And here is how EMMS package is defined (it is quite complex though
comparing with most packages):

<http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/emacs.scm#n897>

¹ https://www.gnu.org/software/guix/

--
Alex

Yoni Rabkin

2016-12-04 21:31:59 UTC

I confirm, also 'make' doesn't build emms-print-metadata, because it is
not specified in "all" target.

Thank you for this.
I've fixed this and it will be released along with version 4.2 in
November.

I see that you didn't add 'emms-print-metadata' to 'all' target in
commit cbbe098¹, so "make" still doesn't build it. Was it intentional?

Yes. At this point we still want to keep the make phase to be about
elisp. This should be resolved when we decide how to do the compilation
in way compatible with remotely installable elisp packages.

Sorry, I don't understand; do you mean installation from ELPA? But it
doesn't care about Makefile at all, so I don't see what problem can be
caused by adding 'emms-print-metadata' to 'all' target.

I don't know what the right solution is right now. I'll have to ask the
emacs-devel people. I don't know of an existing solution that makes
sense to me. A pdftools-like solution seems too inelegant. And
installing a binary on the user's system by default doesn't sound right
to me when they are compiling and installing an elisp package; it isn't
expected behavior.

Perhaps a message can appear at the end of elisp compilation telling
users that they can also invoke make emms-metadata and install that, as
well as pointing people to the friendly manual.

But then I'm not even a fan, nor user, of ELPA... so perhaps it isn't
surprising that I'm dithering on the subject.

Do you have access to those systems? Can you do a test to see that it
would work as intended and send a patch?

Yes, I can confirm (I use GuixSD and I am one of the Guix contributors),
but after all it's not a big thing. It's one of the several (small)
issues with the EMMS Makefile that we make workarounds for in our emms
package: the same problem happens with "man" directory (Makefile also
assumes it exists), also info manual is put in "$(PREFIX)/info" instead
of "$(PREFIX)/share/info".
To recap, it is fixed on the Guix side, so you may ignore this message :-)
If you (or someone who is reading) are interested: GNU Guix¹ is a
package manager and GuixSD is a system thereof. It uses Guile
programming language, so people who like sexps may like it too :-)
And here is how EMMS package is defined (it is quite complex though
<http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/emacs.scm#n897>
¹ https://www.gnu.org/software/guix/

I am definitely interested in Guix. Installing it on some machine is on
my todo list.

--
"Cut your own wood and it will warm you twice"

Alex Kost

2016-12-05 09:10:30 UTC

[...]

Post by Alex Kost
I see that you didn't add 'emms-print-metadata' to 'all' target in
commit cbbe098¹, so "make" still doesn't build it. Was it intentional?

Yes. At this point we still want to keep the make phase to be about
elisp. This should be resolved when we decide how to do the compilation
in way compatible with remotely installable elisp packages.

Sorry, I don't understand; do you mean installation from ELPA? But it
doesn't care about Makefile at all, so I don't see what problem can be
caused by adding 'emms-print-metadata' to 'all' target.

I don't know what the right solution is right now. I'll have to ask the
emacs-devel people. I don't know of an existing solution that makes
sense to me. A pdftools-like solution seems too inelegant.

I also don't like the pdf-tools solution, but after all it's a problem
of Emacs package system, as it is intended only to deal with elisp files
(compile them, generate autoloads, etc.), and it also builds texinfo
manuals; but I think there is no good solution to compile binaries with
it.

The author of pdf-tools had to fight with this limitation. As for EMMS,
it works without emms-print-metadata, so if were you I wouldn't even
bother about building emms-print-metadata for ELPA: Emacs package system
is just not suitable for "make"-ing.

Post by Yoni Rabkin
And
installing a binary on the user's system by default doesn't sound right
to me when they are compiling and installing an elisp package; it isn't
expected behavior.

I disagree with this: when I run "make", I expect that the full software
will be built, not just a part of it (I mean only *.el files), so my
opinion is that 'emms-print-metadata' should be built by default, but
you are the boss here :-)

Post by Yoni Rabkin
Perhaps a message can appear at the end of elisp compilation telling
users that they can also invoke make emms-metadata and install that, as
well as pointing people to the friendly manual.

I think it's a good idea... well, building it by default seems a better
idea to me of course :).

--
Alex

Yoni Rabkin

2016-12-05 17:39:33 UTC

And installing a binary on the user's system by default doesn't sound
right to me when they are compiling and installing an elisp package;
it isn't expected behavior.

I disagree with this: when I run "make", I expect that the full
software will be built, not just a part of it (I mean only *.el
files), so my opinion is that 'emms-print-metadata' should be built by
default, but you are the boss here :-)

Well, no. I'm just the current maintainer.

Everyone, please write in with your opinions so that we can decide what
to do with this. If everyone thinks make should compile and install
`emms-print-metadata' then that is what we'll do.

P.S. Alex, how about write access to the Emms git repo?

--
"Cut your own wood and it will warm you twice"

Alex Kost

2016-12-05 20:37:20 UTC