8000 Support pre-compressed uploads in code-intel upload, deprecate LSIF by antonsviridov-src · Pull Request #1146 · sourcegraph/src-cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support pre-compressed uploads in code-intel upload, deprecate LSIF #1146

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

Merged
merged 12 commits into from
Feb 27, 2025

Conversation

antonsviridov-src
Copy link
Contributor
@antonsviridov-src antonsviridov-src commented Feb 20, 2025

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

  • The CLI is now built from source and used to upload the scip-go index for this repository – both in compressed and raw form

@antonsviridov-src antonsviridov-src changed the title WIP Support pre-compressed uploads in code-intel upload, deprecate LSIF Feb 20, 2025
@antonsviridov-src antonsviridov-src force-pushed the allow-compressed-scip-uploads branch from e7234d0 to f9592c2 Compare February 24, 2025 12:05
@antonsviridov-src antonsviridov-src marked this pull request as ready for review February 24, 2025 13:01
@antonsviridov-src antonsviridov-src requested a review from a team as a code owner February 24, 2025 13:01
@varungandhi-src
Copy link
Contributor

As an alternative proposal, I can modify the scip-go Github Action we have to build this CLI from source and use it for upload. WDYT @varungandhi-src?

You mean the one in this repo itself? Yeah, sure, that seems like a good idea. 👍🏽

@antonsviridov-src
Copy link
Contributor Author

Ok, updated the scip-go workflow
Left it as uploading uncompressed index, as this is the most prevalent workflow customers have, so we don't want to break that.

Copy link
Contributor
@varungandhi-src varungandhi-src left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@@ -53,6 +57,7 @@ var (

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`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this flag? Can we just do it purely based on the file extension without it being configurable?

Comment on lines 208 to 211
return "", formatInferenceError(argumentInferenceError{
"file",
errors.Newf("both %s and %s were found, choose the file to upload explicitly using -file flag", scipFilename, scipCompressedFilename),
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author
@antonsviridov-src antonsviridov-src Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonsviridov-src antonsviridov-src merged commit 53e36ab into main Feb 27, 2025
8 checks passed
@antonsviridov-src antonsviridov-src deleted the allow-compressed-scip-uploads branch February 27, 2025 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0