OmniSharp / omnisharp-emacs

Troll coworkers - use Emacs at work for csharp!
GNU General Public License v3.0
513 stars 94 forks source link

"curl: Argument list too long" on Linux #184

Closed cpitclaudel closed 7 years ago

cpitclaudel commented 9 years ago

For large files (> 10k SLOC), curl complains about its arguments list. Changing the definition of omnisharp--get-curl-command-unix to the following fixes it:

(defun omnisharp--get-curl-command-unix (url params)
  "Returns a command using plain curl that can be executed to
communicate with the API."
  (let ((tmp (make-temp-file "omnisharp" nil ".cs")))
    (omnisharp--write-json-params-to-tmp-file tmp (json-encode params))
    `(:command
      ,omnisharp--curl-executable-path
      :arguments
      ("--ipv4" "--silent" "-H" "Content-type: application/json"
       "--data-binary" ,(concat "@" tmp) ,url))))
mikavilpas commented 9 years ago

This has been an issue on Windows for some time, but I haven't encountered this on linux myself. I'll look into it.

Could you open a pull request for this change?

I'm working on adding support for omnisharp-roslyn, which is the new, yet to be released server based on roslyn. It will get rid of curl altogether so this problem will likely be avoided in that version. Nevertheless this needs to be fixed in the current omnisharp-emacs version too.

cpitclaudel commented 9 years ago

Thanks for the quick reply! I'm not sure this warrants a pull request, nor your time investigating it if curl is going away anyway :) I mostly posted this solution out of convenience if someone else was running into this problem, but it's neither configurable nor consistent with the corresponding Windows snippet (btw, I'm not too sure that using a single file name is a good idea; you might run into nasty concurrency issues if two instances of emacs are running).

I used omnisharp-roslyn as the backend.

cpitclaudel commented 9 years ago

Btw, one thing that could work well if you want support for developing the Emacs mode is to create stubs of non-essential functions instead of implementing them, and then pointing them out in the README. If there's a bunch of these you'll save time, and you probably have a good vision of what features need to be implemented at the emacs level. This way you can focus on the architecture issues, and plus the code will be well documented (since you'll just write documentation for these functions, instead of the entire thing).

Thanks for your work on this!

cpitclaudel commented 9 years ago

Btw, speaking of moving away from curl: did you consider the url-retrieve function? It's asynchronous and, from the docs:

(...) The variables `url-request-data', `url-request-method' and
`url-request-extra-headers' can be dynamically bound around the
request
mikavilpas commented 9 years ago

You're right, having multiple emacs instances could be a problem. However, the omnisharp-roslyn branch here on omnisharp-emacs will have the omnisharp-roslyn server as a subprocess and will communicate with its input and output streams. It should get rid of both issues at the same time.

I'll keep your suggestion about having stubbed functions in mind. I'm glad for this kind of constructive conversation! I think there may be lots of users I don't know about since only a few people are visible to me via github.

mikavilpas commented 9 years ago

In the start of this project I tried a few options out, but can't for the life of me remember whether url-retrieve was one of them. If you're curious, you can check out the very first few commits, I documented my research there partially.

https://github.com/OmniSharp/omnisharp-emacs/commits/master?page=24

cpitclaudel commented 9 years ago

Thanks for the pointer to the project history's! It's cool that you're working on this and on this project as a whole.

You're right, having multiple emacs instances could be a problem. However, the omnisharp-roslyn branch here on omnisharp-emacs will have the omnisharp-roslyn server as a subprocess and will communicate with its input and output streams. It should get rid of both issues at the same time.

That sounds cool! I think I can contribute some personal experience here: this approach will work well in most cases, but it could break horribly if you're processing large amounts of text (about 1MB maybe?). The crux of the matter is that, unless you do things the right way, you don't really have a way to say when you're done accepting output. Emacs' comint-mode encourages this to some extent: the idea is that you have a REPL running in the background and after issuing a command you just wait for the next prompt. Of course since the output can come in small batches you accumulate them into a buffer, and then you repeatedly search the accumulation buffer for the next prompt. If that's not done carefully, you end up with quadratic search behaviour (rescanning the entire buffer every time, because the prompt could have been split, so you can't just resume from the end of the last batch of text you received). Even if you implement the forward searching properly, if Emacs decides to send a response in small batches of 20 characters at a time, then overhead of calling your output processing function on each of these small batches can be too costly.

One good way to avoid this kind of behaviour is for the engine to announce how much data its sending (not unlike an HTTP header) (although even then, there can be performance issues with Emacs' pipes). Other things that are generally helpful include disabling undo in the accumulation buffer, looking at the process-adaptive-read-buffering variable, and if you're going to produce a very large file just writing it out to disk and reading it from emacs (that last feature can be supported pretty easily by having two kind of responses, one with a body and one with a "redirect" field that points to a file).

mikavilpas commented 9 years ago

That's extremely useful insight, thanks.

mikavilpas commented 7 years ago

This is quite old now, but now that I look at it, it actually has been useful. The feature-omnisharp-roslyn branch uses a subprocess and all the information you gave me was useful in implementing reading from it.

I'm closing this now as I think it's served its purpose well. Thanks again.