Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add doxymacs to non-GNU ELPA #7

Open
pniedzielski opened this issue Apr 18, 2016 · 11 comments
Open

Add doxymacs to non-GNU ELPA #7

pniedzielski opened this issue Apr 18, 2016 · 11 comments

Comments

@pniedzielski
Copy link
Owner

No description provided.

@nannanmath
Copy link

Hi, I can not find Doxymacs in MELPA.
My config is:

(require 'package)
(add-to-list 'package-archives
  '("melpa" . "http://melpa.org/packages/") t)
(package-initialize)

If I have something wrong?
Thanks.

@pniedzielski
Copy link
Owner Author

It is on our todo list to put doxymacs on MELPA (in fact, you're commenting on our todo list!), but there are a couple things that we need to get done first. Note how this is tagged as an enhancement we want to get done before the 2.0.0 release.

@keinstein
Copy link

As it is tagged as help-wanted: Would you like to describe which kind of help you have in mind?

@pniedzielski
Copy link
Owner Author

@keinstein, I haven't looked at how to add packages to MELPA. It's help-wanted, because it would be a really great thing to do, it's something that isn't high on my priority list, and it's something I think is low-hanging fruit that someone could come in and tackle without much experience with the codebase. That last bit in particular is different from, e.g, adding support for doxypress, which is also on my radar, but which I know will involve some deep changes to the code to get it to the place I want it to be.

@pniedzielski pniedzielski changed the title Add doxymacs to MELPA Add doxymacs to non-GNU ELPA Jun 1, 2023
@L0ren2
Copy link

L0ren2 commented Oct 7, 2023

Hey, I spent some time looking into this, since me and many others on IIRC where
blown away by the fact that a package as popular as this one is not in melpa or
any other emacs package mirror.

I tried to add this to melpa, since to me it seems to have the best instructions
on adding a package. In the end I came up with the following and quite
minimalist melpa/recipes/doxymacs file:

(doxymacs :fetcher github
          :repo "pniedzielski/doxymacs")

This managed to clone the repository as it should but then failed to build the
package.

Problem with current setup

Currently, there exists no doxymacs.el file in the repo's lisp
directory. Thus, when trying to install using melpa's default actions,
installation fails. Furthermore, the current state of the repository cannot be
built (when installing via melpa), since it relies on C code. Several shell
commands must be executed to build the binary from the C code before the package
can be installed and used.

Solution(s)

Quick

Add a file doxymacs.el to the repo, which does nothing more than executing the
below build instructions using (shell-command "<cmd>"). After building, the
actual elisp code needs to be loaded as well. This solution is inspired by the
way the evil package lays out its source code (one file evil.el loading all
other elisp files).

Build instructions of AUR -> PKGBUILD since this repo's INSTALL appears to be outdated

./bootstrap
./configure --prefix=</path/to/install/dir>
make
make install

If specifying /usr as prefix argument to configure, /usr/bin will be the
chosen install path (bin gets appended).

Not so quick

Rewrite the C portion of the code in elisp and include it in the appropriate
places. Have one elisp file that contains the actual package code and include
the rest of the former C code. Note that this is only an option if the marginal
performance boost of C code compared to byte-compiled elisp is not a necessity.

I would be willing to conduct further experiments to try and get it to work using a fork.
That is, if you really want to wait for me creating a pull request. It took me just shy of
three weeks to get to where I am right now, so you might be faster trying it out yourself.

Greetings

L0ren2 added a commit to L0ren2/doxymacs that referenced this issue Oct 7, 2023
one step into the direction of the "Quick" fix mentioned in pniedzielski#7
@pniedzielski
Copy link
Owner Author

@L0ren2

First, thank you for your investigation and detailed write-up. This gave me the information I needed!

Your quick solution

I took a look at your fork with your quick solution. While it should do the job, it does mean that every time the package is loaded, we recreate our configure script, our Makefile, our doxymacs-impl.el, and our C parser, and we (potentially) require running Emacs as superuser to install the C parser. While make will save us from doing a full compile each time, it still doesn't seem like a good thing to me to run shell commands on the user's system so eagerly and sneakily.

The path pdf-tools takes is to let the user explicitly choose when to compile the package's C component, which seems better to me. The user has to opt-in.

(Also, the build instructions currently are for running from a packaged version, created by prepare-release.sh. It's only when building from Git that you need to run bootstrap.sh (and so I guess for MELPA, too, then). For packaged versions, there is no bootstrap.sh, which is why AUR gives different build instructions from us. It's a good change in the docs to make this more explicit, but the build instructions are as intended.)

Getting rid of autotools, etc

To me the biggest issue you've found is not the C program, but autotools. If we can get rid of our autoconf setup in its current form, we'll be in a much better position to put the package on ELPA. There are a few parts to this, some independent:

Currently we use autoconf both to substitute in some variables in the doxymacs.el output file and to build an XML parser in C. On the former front, most of those variables we can almost certainly get rid of---the VERSION, the DEFAULT_STYLE, etc. See configure.ac for this. This is probably independently a good thing to do. The version changes can be automated with CI, and the DEFAULT_STYLE seems needless (it can already be configured in Emacs, so any default we pick is fine). Finally, to remove the path to the external C parser, we can use the technique pdf-tools uses. This all seems easy, and means the only need for autoconf is to build the external C parser. For users who don't want to use the external C parser, they don't need to use autotools at all, which means we can package this for ELPA without worry. And, if we use that trick I mentioned above of using (doxymacs-install) or such to run autotools, we can allow the user to control when or if they compile the external C parser at all.

Now, as you suggested we can go further and remove the C code. For the XML parser in C, I've been hoping to get rid of this for a while (issue #16), but two things have kept me back:

  1. There are two parsers in the project right now: there's a native elisp implementation, and there's the C implementation. The C implementation is significantly more memory efficient than the elisp implementation; this is where the performance gain comes from. This is important, as Doxygen XML files can get very, very large (GBs). I don't think byte-compiling or not will enter into it. The C code was written twenty years ago, though, when this was not simply an optimization, but was necessary for the package to function on medium-to-large-sized projects. It may no longer be necessary, but I want to make sure I have benchmarks for that now.
  2. Emacs does have built-in libxml2 support now, which is what the C code uses. This will help with speed, but not memory usage, as it will construct an elisp object for the entire DOM. This may be okay, but before removing the C code, I would like to be sure. Ideally, for such large files, a SAX parser would be better than a libxml2 style parser, but that may only be possible in C.

This is the way I'd like to go, but it may be worth putting the package in (M)ELPA before removing the C code. This way, at least, anyone who's working on a large project that does need the external C parser will not have the rug pulled out from under them.

MELPA vs non-GNU ELPA

While MELPA is the biggest package archive, I think it may be best to target non-GNU ELPA, because that will put this package in the hands of all Emacs users by default, and I can control the release cycle. The instructions for adding a package to non-GNU ELPA are different, though not terribly hard. But, I don't think most of what you've found depends strongly on that MELPA script.

It is possible to add it to both, and some packages do that, but I'll have to be very careful with versioning.

Next steps

If you're interested in pursuing this, absolutely! If not, I can start on removing the elisp side of autotools and adding a (doxymacs-install) command to build the C code. Either way, I'm interested in getting this package in non-GNU ELPA, and I think we now have a path forward. Thanks!

@L0ren2
Copy link

L0ren2 commented Oct 12, 2023

Hey, @pniedzielski
Sorry for responding a bit late. Sadly I've had a busy schedule lately. I'm
happy I could help a little bit.

MELPA was the first thing that I stumbled upon, which was why I thought it might
be a good choice. I now read about the versioning implications of MELPA.
Knowing that, non-GNU ELPA might be the better choice after all.

I would love to continue this saga on helping doxymacs into the package
repositories and would like to look into how to add packages to non-GNU ELPA as
my next step.

It would be fantastic if you could have a look at the options at our disposal to
get rid of autotools and patch the buildsystem so that the users can opt-in to
compilation, since you know about the codebase and what autotools do and also
how autotools works in general. I'm not very much into coding elisp right now
because I've just been using Emacs for the last two months now with no real
prior experience. I have worked a little with clisp back at college but it will
take me some time to get up to speed with lisp again, so It makes sense for you
to handle that as of now.

The spontaneous and sneaky execution of shell commands you mentioned were a
first-quite misguided-shot in that direction. I was not able to build the
package using the instructions in the INSTALL file, but that is probably due to
my lack of experience using autotools. It was for that reason I reverse
engineered the AUR build script.

I will keep you updated if I find out anything new and I'm glad to work together
with you on this issue :D

@L0ren2
Copy link

L0ren2 commented Oct 15, 2023

Hey, @pniedzielski
I had a look at the nonGNU ELPA package repo by now.
My fork currently holds a compatible layout, that we could get into the nonGNU ELPA if we wanted to. Or at least I think so. Out of the box, it does build, but only the elisp files. I attached a makefile for easier building of the doxymacs_parser.c file.

To get where this is right now, I needed to ditch automake in the process of refactoring the repository structure once more; Instead, a Makefile can be used to build all source files. I first built the package using the INSTALL instructions, then copied the source files where I thought they should be to build the package using nonGNU ELPA's build system and finally removed all automake related stuff.

Configuring the build in a fine-grained manner is thus no longer an option, but I don't know how many people made use of that feature beforehand. Furthermore I have not tested building the package using XEmacs, since it is not supplied by my package manager and I was too lazy to build it myself because it's also not in the AUR either. The INSTALL and README files where updated according to the usage of the Makefile.

To get the doxymacs repository a bit cleaner, we could think about merging the Copy-license files into one file that tells users that redistribution is possible under the conditions of GPLv2+.
Anyways, I think it would be cool if you could have a short look at my fork and tell me if something is wrong, so we can get this shipped :)

@pniedzielski
Copy link
Owner Author

@L0ren2 I'm just giving you an ACK; I'll have a chance to look at your fork this evening, and we can move forward with this. If it looks good overall, you can make a PR, I can take a look at the details, and we can get the ball moving on submitting this to nonGNU ELPA.

Thanks again for your work on this!

@pniedzielski pniedzielski self-assigned this Oct 18, 2023
@L0ren2
Copy link

L0ren2 commented Oct 25, 2023

Hey, I just posted a pull request after making some final changes on my end.
Hope this works for you. As yo can tell by the title of the pull request, I'd like to continue working on this awesome project!
Thanks for having me.

@L0ren2
Copy link

L0ren2 commented Nov 6, 2023

Hey,
Did you have a chance to look at the pull request yet?
I noticed two things that I wanted to fix, but that I did not yet get around to do:

  • I wanted to add an install rule to the makefile
  • I remembered that I wanted to test C-c d ? but I forgot to do so before posting the PR and have still not had the time to do so
    If everything else is alright, I can fix these two things and post another PR if that is fine with you.
    Let me know if you find something you don't like :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants