Browsing pull request fails with cl-no-applicable-method

[dod38fr]

Hi

Since I've upgraded to forge-20240219.1113 (from 20240117-1701), browsing pull request (N b p command) fails with cl-no-applicable-method. The pull request are done on an on-premise GitHub server. I do not know yet if this behavior can be reproduced with github.com

All the best

backtrace:

Debugger entered--Lisp error: (cl-no-applicable-method forge-get-topic nil)
  apply(debug (error (cl-no-applicable-method forge-get-topic nil)))
  transient--exit-and-debug(error (cl-no-applicable-method forge-get-topic nil))
  signal(cl-no-applicable-method (forge-get-topic nil))
  cl-no-applicable-method(#s(cl--generic :name forge-get-topic :dispatches ((1 #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>))) (0 #s(cl--generic-generalizer :name eieio--generic-generalizer :priority 50 :tagcode-function cl--generic-struct-tag :specializers-function #f(compiled-function (tag &rest _) #<bytecode 0xd6a5a144e7c4343>)) #s(cl--generic-generalizer :name cl--generic-typeof-generalizer :priority 10 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x4b524acc11b73a2>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x1a9191698282249c>)) #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>)))) :method-table (#s(cl--generic-method :specializers (forge-notification) :qualifiers nil :call-con nil :function (closure (t) (notify) (progn (let* (...) (if repo ...))))) #s(cl--generic-method :specializers (forge-pullreq-post) :qualifiers nil :call-con nil :function (closure (t) (post) (progn (forge-get-pullreq post)))) #s(cl--generic-method :specializers (forge-issue-post) :qualifiers nil :call-con nil :function (closure (t) (post) (progn (forge-get-issue post)))) #s(cl--generic-method :specializers (string) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (id) (progn (or (forge-get-issue id) (forge-get-pullreq id))))) #s(cl--generic-method :specializers (integer) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (number) (progn (if (< number 0) (forge-get-pullreq ...) (or ... ...))))) #s(cl--generic-method :specializers (forge-repository t) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (repo number-or-id) (progn (if (numberp number-or-id) (if ... ... ...) (or ... ...))))) #s(cl--generic-method :specializers (forge-topic) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (topic) (progn topic)))) :options nil) nil)
  apply(cl-no-applicable-method #s(cl--generic :name forge-get-topic :dispatches ((1 #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>))) (0 #s(cl--generic-generalizer :name eieio--generic-generalizer :priority 50 :tagcode-function cl--generic-struct-tag :specializers-function #f(compiled-function (tag &rest _) #<bytecode 0xd6a5a144e7c4343>)) #s(cl--generic-generalizer :name cl--generic-typeof-generalizer :priority 10 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x4b524acc11b73a2>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x1a9191698282249c>)) #s(cl--generic-generalizer :name cl--generic-t-generalizer :priority 0 :tagcode-function #f(compiled-function (name &rest _) #<bytecode -0x1cba0b13a96764e4>) :specializers-function #f(compiled-function (tag &rest _) #<bytecode -0x2b662d08469105e>)))) :method-table (#s(cl--generic-method :specializers (forge-notification) :qualifiers nil :call-con nil :function (closure (t) (notify) (progn (let* (...) (if repo ...))))) #s(cl--generic-method :specializers (forge-pullreq-post) :qualifiers nil :call-con nil :function (closure (t) (post) (progn (forge-get-pullreq post)))) #s(cl--generic-method :specializers (forge-issue-post) :qualifiers nil :call-con nil :function (closure (t) (post) (progn (forge-get-issue post)))) #s(cl--generic-method :specializers (string) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (id) (progn (or (forge-get-issue id) (forge-get-pullreq id))))) #s(cl--generic-method :specializers (integer) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (number) (progn (if (< number 0) (forge-get-pullreq ...) (or ... ...))))) #s(cl--generic-method :specializers (forge-repository t) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (repo number-or-id) (progn (if (numberp number-or-id) (if ... ... ...) (or ... ...))))) #s(cl--generic-method :specializers (forge-topic) :qualifiers nil :call-con nil :function (closure (vertico--input vertico-mode t) (topic) (progn topic)))) :options nil) nil)
  #f(compiled-function (&rest args) #<bytecode -0x1dc87bcec24497a7>)(nil)
  apply(#f(compiled-function (&rest args) #<bytecode -0x1dc87bcec24497a7>) nil nil)
  forge-get-topic(nil)
  (let ((obj (forge-get-topic topic))) (browse-url (forge-get-url obj)) (forge-topic-mark-read obj))
  forge--browse-topic(nil)
  (closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request))(nil)
  apply((closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request)) nil)
  (let ((debugger #'transient--exit-and-debug)) (apply fn args))
  (unwind-protect (let ((debugger #'transient--exit-and-debug)) (apply fn args)) (let* ((unwind (and t (eieio-oref prefix 'unwind-suffix)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))
  (closure ((advice lambda (fn &rest args) (interactive (closure (#3 (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (spec) (let (...) (unwind-protect ... ...)))) (apply '#0 fn args)) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (fn &rest args) (unwind-protect (let ((debugger #'transient--exit-and-debug)) (apply fn args)) (let* ((unwind (and t (eieio-oref prefix ...)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil)))((closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request)) nil)
  apply((closure ((advice lambda (fn &rest args) (interactive (closure (#4 (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (spec) (let (...) (unwind-protect ... ...)))) (apply '#1 fn args)) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (fn &rest args) (unwind-protect (let ((debugger #'transient--exit-and-debug)) (apply fn args)) (let* ((unwind (and t (eieio-oref prefix ...)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) (closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request)) nil)
  (lambda (fn &rest args) (interactive (closure ((advice . #0) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (spec) (let ((abort t)) (unwind-protect (prog1 (let ... ...) (setq abort nil)) (if abort (progn ... ... ...)))))) (apply '(closure ((advice . #0) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (fn &rest args) (unwind-protect (let (...) (apply fn args)) (let* (...) (if unwind ... nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) fn args))((closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request)) nil)
  apply((lambda (fn &rest args) (interactive (closure ((advice . #1) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (spec) (let ((abort t)) (unwind-protect (prog1 (let ... ...) (setq abort nil)) (if abort (progn ... ... ...)))))) (apply '(closure ((advice . #1) (suffix . forge-browse-pullreq) (prefix . #<transient-prefix transient-prefix-15611a1d9c52>)) (fn &rest args) (unwind-protect (let (...) (apply fn args)) (let* (...) (if unwind ... nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) fn args)) (closure (t) (pull-request) "Read a PULL-REQUEST and visit it using a browser.\n..." (interactive (list (forge-read-pullreq "Browse pull-request"))) (forge--browse-topic pull-request)) nil)
  forge-browse-pullreq(nil)
  funcall-interactively(forge-browse-pullreq nil)
  command-execute(forge-browse-pullreq)

[tarsius]

forge--browse-topic(nil) indicates that forge-read-pullreq returned nil, which I don't think is possible with the current version. I suspect you loaded forge, and then updated forge without restarting emacs, ending up with a franken version. Have you tried turning it off and on again?

[dod38fr]

Yes. Several times.

I'm now back to forge-20240213.2352.

When working on a know repo, forge is now telling me that "Forge doesn't know about this Git repository yet". Looks like something is wrong with the DB content.

The repo is named "core-platform-aws" and sqlite shows:

sqlite> select * from repository where name like "%platform-aws%";
github|"c2NobmVpZGVyOjAxMDpSZXBvc2l0b3J5NDExNTA="|"MDEwOlJlcG9zaXRvcnk0MTE1MA=="|"[redacted]"|"[redacted]Platform"|"core-platform-aws"|"github.[redacted].com/api/v3"|"github.[redacted].com"|"origin"||"2023-06-30T09:43:58Z"|"2023-10-18T13:40:11Z"|||"AWS core monorepo"|"https://pages.github.[redacted].com/[redacted]Platform/core-platform-aws/"|"master"|||||t|t||0|0|eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound||"/home/domi/aws-work/core-platform-aws/"|eieio-unbound|"2024-02-09T03:18:30Z"|"2024-02-09T11:29:05Z"
github|"Z2l0aHViLnNjaG5laWRlci1lbGVjdHJpYy5jb206MDEwOlJlcG9zaXRvcnk0MTE1MA=="|"MDEwOlJlcG9zaXRvcnk0MTE1MA=="|"github.[redacted].com"|"[redacted]Platform"|"core-platform-aws"|"github.[redacted].com/api/v3"|"github.[redacted].com"|"origin"|t|||||||||||||t||||eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound|eieio-unbound||"/home/domi/aws-work/core-platform-aws/"|eieio-unbound||

Looks like DB content is weird. May be an upgrade issue.

Should I delete the DB and restart from scratch ?

All the best

PS: edited to delete the part about select requiring like, the name column in DB does contain surrounding quotes, i.e. "core-platform-aws" and not core-platform-aws

[tarsius]

Yes. Several times.

I cannot assume that users have already done that because "they are Emacs users, not office drones, so surly they must already have done that". You would be surprised how often my telling someone to do that, fixes the issue they were having. I would say at least once a week on average.

If that fails, the next step is to instruct them(/you) to do the re-installation dance: Uninstall all relevant packages (here forge should do, but throw in closql and ghub for good measure), exit emacs, start emacs again, install these packages again.

In the case of Forge, I think it is a bad idea to try to work around an issue by downgrading. I am making changes to the db schemata, and while I provide an upgrade path, going the other way is not possible. Usually I bump the database "version" when making changes, and if you try to use such a db with an older version of forge you should get an error, instructing you to upgrade forge to a compatible version. Maybe I forgot that here or "something went wrong".

Should I delete the DB and restart from scratch ?

At least for debugging purposes, this sounds like a good idea. [Because you previously downgraded. At this time I don't think there's an issue with the database, but you could have introduced one by downgrading, which would mean there are two issues now, which of course has the potential of making it much harder to investigate the original issue.]

But first update to the newest version of forge again.

PS: edited to delete the part about select requiring like, the name column in DB does contain surrounding quotes, i.e. "core-platform-aws" and not core-platform-aws

Yes, Emacsql works like this see https://github.com/magit/emacsql?tab=readme-ov-file#why-are-all-values-stored-as-strings

So in summary, get back to the state we were in before and then let's find out why forge-read-pullreq is capable of returning nil for you even though I think that "isn't actually possible". Maybe there's a bug in the latest version of that function; maybe it is because you are using an old version of that function. It was never supposed to be able to return nil but actually could do it, due to a bug!

This is the reason I suspect you might be using an old version of Forge: I know that this function could return nil until very recently and I fixed that. And now you are presenting me a backtrace that demonstrates that it does in fact return nil.

The code that should now prevent you from "selecting nil" boils down to (magit-completing-read "test" '("a" "b" "bb") nil t) (specifically the t in there); if you try to exit with empty input you get an user-error: Nothing selected.

That code is in forge--read-topic. Use find-function to jump to the definition of that function. If that fails, we know you are using a version of forge, that doesn't have that function yet.

[tarsius]

Does N b p actually prompt you for a pull-request? Depending on configuration it could also just act on the "current" pull-request. I haven't really thought about what could go wrong in that case yet.

[dod38fr]

ok, I've re-installed latest version, deleted current DB and restarted from a V11 database. I cannot reproduce the mess I had before, so I may have run a franken version of forge for a few minutes before thinking of restarting (I guess we'll never know)

Using V11 DB, it looks like forge has forgotten about already configured repositories (they are unknown from forge). Once I've re-imported the repo, N b p works fine (and prompt for a pull-request).

Having to re-import repo is annoying, but I can live with that.

Thanks for the detailed explanations.

[tarsius]

Having to re-import repo is annoying, but I can live with that.

You might want to give forge-add-user-repositories and forge-add-organization-repositories a try. (Note though that they are far from perfect. I might make them more robust eventually.)

[dod38fr]

fair enough. Sorry for the silence, I was on vacation. I'll re-open if I manage to reproduce this behavior.

[bcc32]

I have observed this error recently for a github.com repo. I also saw forge-read-pullreq return nil, even though I am on a very recent version of forge (database version 13):

Package forge is dependency.

     Status: Installed in ‘forge-20240320.2049/’ (unsigned).
    Version: 20240320.2049
     Commit: f9eec752654120a3f0b5151ff0e141a37443bcce
    Summary: Access Git forges from Magit.

In my case, I am trying to use forge-visit-pullreq to look at some pull requests I recently merged. The active pull requests seem to work fine; it's only the merged ones that are broken. (In fact, the merged ones don't even show up in the completion list unless point is on the line listing them in the magit-status buffer.)

Happy to help provide any other information if that would be helpful.

[tarsius]

In fact, the merged ones don't even show up in the completion list unless point is on the line listing them in the magit-status buffer.

This is intentional. Only open pull-requests and the pull-request at point are offered as completion candidates. That's because it is more likely that the user wants to select one of those prs than one of the closed ones. Offering all prs as candidates anyway, would have two downsides; it becomes harder to spot the desired pr based on its description and when there are a large number of prs, this also has a performance impact.

But of course one should be able to select a closed pr when that is desired, that's why the prompt reads: View pull-request (+ for all):, i.e., type + to extend the set of completion candidates to include all prs.

I also saw forge-read-pullreq return nil,

forge--read-topic passes t for (magit-)completing-read's require-match argument to prevent the user exiting with invalid input, so that callers can rely on only valid values being returned. Unfortunately, when a function is used as collection, as we do here, then the completion functionality ignores this instruction.

I will have to read relevant code to see if it is possible to work around this defect.

[tarsius]

It seems the defect is only that it's not mentioned that one gets to reimplement this whenever one want to use a function as collection. But that should be doable.

[bcc32]

Does the + key only work with vertico? I experienced the following (after updating forge to dbfbfa71523ff1a0ece89d823ee7450c4123ffd9):

1. forge-visit-pullreq errors when it tries to read the vertico-mode variable, if vertico is not installed.
2. I installed vertico and disabled ivy-mode, enabled vertico-mode, and made sure magit-completing-read-function was set to magit-builtin-completing-read
3. I invoked forge-visit-pullreq while point was on a merged pull request. The merged pull request was included in the completion candidates, but selecting it signaled cl-no-applicable-method: No applicable method: forge-get-pullreq, nil
4. I invoked forge-visit-pullreq while point was on a merged pull request, then pressed +. Only then did selecting it correctly show me the pull request.

[bcc32]

I also just noticed that, if forge-limit-topic-choices is set to nil, forge--read-topic will only show the active topics, as opposed to showing all topics like the customization's docstring says it will.

[tarsius]

Oh boy...

I've fixed these bugs too now (knock on wood). Thanks a lot, your report was very useful!

[bcc32]

Working for me now, thanks for the quick fix!