Improve package.el compatibility

[jabranham]

I think the commit messages are self-explanatory, let me know if there are any questions.

Related to #2

[vspinu]

Finally someone got down to doing the dirty work :) 🥇 I don't see any issues with the changes.

Have you tried it with local melpa install? All works as expected?

[jabranham]

Have you tried it with local melpa install?

No, and I'm not really sure how to do so. If no one else is, I can read up on how to do this.

All works as expected?

As far as I can tell, there's no user-visible changes other than some nicer autoloading of features. Which people won't notice if they're not autoloading ess anyway.

[jabranham]

Hrm... playing around with MELPA is giving me errors. I guess hold off on merging this until I figure out what it's doing.

[jabranham]

I can get this to work correctly with MELPA if I change the melpa "recipe" file to:

(ess :repo "jabranham/ESS" :fetcher github :branch "improve-package.el-compatibility" :files
     ("*.el" "lisp/*.el" ("etc" "etc/*")
      "doc/*.texi" "doc/info/dir"))

so it looks like we might have to coordinate with them in order to change the :files part when/if this gets merged.

[jabranham]

And as a bonus, package.el users do not need to (require 'ess-site) anymore. Everything is properly deferred and autoloaded so that after (package-initialize) is called (which I think Emacs does automatically now?) visiting an R file loads ESS automatically.

[vspinu]

Are the errors due to ess-pkg.el? If so we can leave it empty till new recipe is merged into melpa.

[jabranham]

No, they're because I removed ess-autoloads.el (which we're not supposed to have according to the Elisp manual). That file added the lisp/ subdirectory to load-path. Now that it's gone, lisp/ isn't getting added.

Changing the ess recipe to what I did above solves this by just bringing all the lisp/*.el files into the top-level. It's similar to what magit does (magit also has their el files in a lisp subdir).

[vspinu]

That file added the lisp/ subdirectory to load-path.

I recall that that was a hack before package.el could deal with nested directories.

Changing the ess recipe to what I did above solves this

Ok. I will try to find time to check this locally tomorrow and will send a PR to melpa once it's merged.

[lionel-]

nice!

[jabranham]

Any reason not to merge this? @vspinu

[vspinu]

I couldn't find time to figure this out yet. I want to get into details of package.el on this occasion and check melpa locally. Sorry about the delay :(

[lionel-]

I'll look into it as well to sort out the Makefile issues (cf the other PR). Hopefully there's a way of preventing compilation of some files.

[jabranham]

@vspinu - no problem, and no rush really. Let me know if I can answer any questions!

@lionel- AFAIK there's no way except through no-byte-compile. I'll double-check MELPA though. Why wouldn't we want to byte compile all files, though?

[lionel-]

Because some files depend on macros that only exist in recent Emacsen, e.g. flymake-log in ess-r-flymake.el. If you compile with an old Emacs and then run with a recent one (admittedly a non optimal situation but not an unrealistic one) you get really anoying errors when using ESS.

[jabranham]

Ah, I hadn't thought of that, makes sense. And I see the other PR you referenced earlier. I should've checked that, sorry for making you repeat yourself.

As far as I know, there's not an easy solution (other than having the user re-compile, anyway). I'll think about it some more though....

FWIW I don't use the Makefile ESS provides. I just byte compile everything in lisp/, add it to load-path, and update the autoloads. This is more or less the way package.el handles things.

I wonder if ESS should move to suggesting installation via package.el for everyone, since that's the "official" way of providing Emacs extensions. But that's a bit off topic for now.

[jabranham]

OK to add (add-to-list 'load-path (file-name-as-directory (concat ess-lisp-directory "/obsolete"))) to ess-site in this PR so that we can obsolete things as discussed in #547 ?

[vspinu]

Yes please. I am still struggling to find time for this, sorry. Will do my best to sink into by the end of the week.

[jabranham]

No rush.

If it's helpful, here's how I've been working with melpa locally:

git clone git@github.com:melpa/melpa.git
cd melpa 
make recipes/ess

that creates packages/ess-<date>.<number>.tar, which you can install in a fresh Emacs using package-install-file (NB that this puts it into ~/.emacs.d/elpa/ess<blah>).

If you change recipes/ess to this:

(ess :repo "jabranham/ESS" 
  :fetcher github 
  :branch "improve-package.el-compatibility" 
  :files
     ("*.el" "lisp/*.el" ("etc" "etc/*") ("obsolete" "obsolete/*.el")
      "doc/*.texi" "doc/info/dir"))

then it will use this branch to build the recipe instead of master. You can then use it using package-install-file. You shouldn't need to (require 'ess-site) anymore, since package.el should set up the autoloads needed to activate ESS in appropriate situations.

Let me know if you run into any problems or have questions!

[jabranham]

@vspinu have you had a chance to look at this?

[vspinu]

Not yet. Sorry. I have had a bunch of rough weeks :/ Cannot do it this weekend but for sure on Monday or Tuesday.

[jabranham]

No problem, thanks!

…

On Sat, Jun 9, 2018, 4:18 PM Vitalie Spinu ***@***.***> wrote: Not yet. Sorry. I have had a bunch of rough weeks :/ Cannot do it this weekend but for sure on Monday or Tuesday. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#543 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALXMiWLskf1M-gpaNZwy8NAaBYUFLuVRks5t7DuVgaJpZM4T91Nj> .

[jabranham]

Just rebased off master and there is a failing test. It's to do with that font lock stuff I mentioned when adding this test yesterday. Weirdly, it only fails on emacs-git. I've no idea why.

@lionel- perhaps font lock info should be made optional in these tests?

[vspinu]

Most people won't be able to simply remove (load "ess-site.el") after this. Manipulating any variable or alias (including plenty of recent obsoletes) which is not autoloaded will trigger an error.

The good thing is that most people will still have (load "ess-site") even when loading from MELPA. I propose that we keep all our obsoletes at the end of their files and alias them.

[jabranham]

Manipulating any variable or alias (including plenty of recent obsoletes) which is not autoloaded will trigger an error.

What do you have in mind here? I'm assuming most people aren't doing much outside e.g. (setq ess-some-var some-val).

If they do something more complicated, they can always wrap in in with-eval-after-load.

[vspinu]

I was changing ess-imenu-S-generic-expression. I bet there will be guys manipulating ess-r-customize-alist directly. But you are right, whoever does that should be able to figure with-eval-after-load.

[vspinu]

I have looked at this and it works as expected in my tests. If there no further proposals I will PR to MELPA tomorrow with the following recipe:

(ess
 :repo "emacs-ess/ESS" 
 :fetcher github 
 :files ("*.el" "lisp/*.el"
         "doc/*.texi" "doc/info/dir"
         ("etc" "etc/*")
         ("obsolete" "obsolete/*.el")
         (:exclude "etc/other")))

[vspinu]

@jabranham please don't rebase or add anything here. I have pulled your fork locally and already started building on top of it. Will merge once MELPA recipe is accepted.

[jabranham]

Great, thanks!

I think we can close #2 after this gets merged?

[vspinu]

There is a pretty long back log for MELPA submissions. It might take a week or two, especially given that our repo is not the cleanest one. They have tightened requirements quite a bit (compilation, checkdoc, package-lint). We are long way from a clean build, but I think 19.04 can be a doable target for an absolutely clean repo.

[jabranham]

I'm guessing they'll be a bit more lenient with ESS since it's an established package. Hopefully we hear back sometime in the next day or two.