Support pre-compressed uploads in code-intel upload, deprecate LSIF #1146
Support pre-compressed uploads in code-intel upload, deprecate LSIF #1146antonsviridov-src merged 12 commits intomainfrom
Conversation
antonsviridov-src
changed the title
antonsviridov-src
force-pushed
the
allow-compressed-scip-uploads
branch
from
e7234d0 to
f9592c2
Compare
You mean the one in this repo itself? Yeah, sure, that seems like a good idea. 👍🏽 |
|
Ok, updated the scip-go workflow |
varungandhi-src
left a comment
varungandhi-src
left a comment
There was a problem hiding this comment.
I have not reviewed the vendored code, but let's make sure we add some comments here and in the monorepo to make it clear which code is coupled to which other code (e.g. using NOTE(id: blah) which can be used for searching)
Mostly non-blocking/stylistic comments. Please @ mention me if you want me to take a deeper/second look at something specific.
cmd/src/code_intel_upload_flags.go
Outdated
|
|
||
| func init() { | ||
| codeintelUploadFlagSet.StringVar(&codeintelUploadFlags.file, "file", "", `The path to the SCIP index file.`) | ||
| codeintelUploadFlagSet.BoolVar(&codeintelUploadFlags.gzipCompressed, "gzip", false, `Treat uploaded file as already gzip compressed`) |
There was a problem hiding this comment.
Why do we need this flag? Can we just do it purely based on the file extension without it being configurable?
cmd/src/code_intel_upload_flags.go
Outdated
| return "", formatInferenceError(argumentInferenceError{ | ||
| "file", | ||
| errors.Newf("both %s and %s were found, choose the file to upload explicitly using -file flag", scipFilename, scipCompressedFilename), | ||
| }) |
There was a problem hiding this comment.
Let's not emit an error for ambiguity? I think the most likely situation is that the user compressed index.scip and created index.scip.gz without deleting the index.scip file. That doesn't warrant a hard error.
If both files are found, let's pick the compressed file and emit an info-level log message instead.
There was a problem hiding this comment.
2 participants
Closes GRAPH-1046
Contains a lot of code vendored in from the public snapshot – the main upload methods were unfortunately private so I had to unravel a bunch of dependencies.
Test plan