Skip to content

Conversation

@klalumiere
Copy link
Contributor

We started to use the dependencygraph internally. It worked for some of our repositories, but for some other, it failed with no message, even when using --debug:

" 2>&1) # last line of the popen call
[DEBUG] 
Dependency graph submission failed.

To understand the issue, I tried running the command curl directly (in a GitHub workflow) and I got curl: Argument list too long, which hinted that maybe we were hitting a limit on the command length with popen. I made the modifications I suggest in this pull request, and this indeed resolve the issue:

[DEBUG] 1288: popen( curl -w "\\nfcfad8a3-bb68-4a54-ad00-dab1ff671ed2%{http_code}" -X POST -H "Accept: application/vnd.github+json" -H "Authorization: ***" -H "X-GitHub-Api-Version: 2022-11-28" https://api.github.com/repos/<A_REPOSITORY>/dependency-graph/snapshots -d @/tmp/vcpkg/dependency_snapshot_9f868846-cd35-4af6-a062-2cd4a5d6b152.json 2>&1)
[DEBUG]   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
[DEBUG]                                  Dload  Upload   Total   Spent    Left  Speed
[DEBUG] 
[DEBUG]   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
[DEBUG] 100  126k  100   227  100  126k    540   300k --:--:-- --:--:-- --:--:--  301k
[DEBUG] {
[DEBUG]   "id": <AN_ID>,
[DEBUG]   "created_at": "2023-07-31T20:54:53.465Z",
[DEBUG]   "result": "ACCEPTED",
[DEBUG]   "message": "The snapshot was accepted, but it is not for the default branch. It will not update dependency results for the repository."
[DEBUG] }
[DEBUG] 
[DEBUG] 1288: cmd_execute_and_stream_data() returned 0

@BillyONeal
Copy link
Member

BillyONeal commented Aug 1, 2023

Stupid question: Could #1134 fix this rather than needing to write a temporary file? (As in, can curl use stdin for this purpose?)

@klalumiere
Copy link
Contributor Author

klalumiere commented Aug 1, 2023

@BillyONeal

Stupid question: Could #1134 fix this rather than needing to write a temporary file? (As in, can curl use stdin for this purpose?)

Yeah I also thought about using stdin. 🙂 curl can use stdin for this purpose with the option -d @-.

I tested it this morning. I started from the branch BillyONeal:process-stdin and I modified send_snapshot_to_api so that it uses stdin. Here's the very small diff between BillyONeal:process-stdin and klalumiere:process-stdin-plus-fix-command-line-too-long.

It works. In a Ubuntu 22.04 container running on a Ubuntu 20.04 host, we get:

[DEBUG] 1288: execute_process(curl -w "\\nfcfad8a3-bb68-4a54-ad00-dab1ff671ed2%{http_code}" -X POST -H "Accept: application/vnd.github+json" -H "Authorization: ***" -H "X-GitHub-Api-Version: 2022-11-28" https://api.github.com/repos/<A_REPOSITORY>/dependency-graph/snapshots -d @-)
[DEBUG]   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
[DEBUG]                                  Dload  Upload   Total   Spent    Left  Speed
[DEBUG] 
[DEBUG]   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
[DEBUG] 100  126k  100   227  100  126k    474   263k --:--:-- --:--:-- --:--:--  264k
[DEBUG] {
[DEBUG]   "id": <AN_ID>,
[DEBUG]   "created_at": "2023-08-01T14:26:32.030Z",
[DEBUG]   "result": "ACCEPTED",
[DEBUG]   "message": "The snapshot was accepted, but it is not for the default branch. It will not update dependency results for the repository."
[DEBUG] }
[DEBUG] 
[DEBUG] 1288: cmd_execute_and_stream_data() returned 0 after   488006 us
Dependency graph submission successful.

I guess it should work on Windows too (I don't know the details about stdin enough on Windows to know if somehow, there's a maximum input length on stdin on this platform).

What are the next steps? Do you want to include these 2 lines in your pull request?

@BillyONeal
Copy link
Member

I tested it this morning. I started from the branch BillyONeal:process-stdin and I modified send_snapshot_to_api so that it uses stdin. Here's the very small diff between BillyONeal:process-stdin and klalumiere:process-stdin-plus-fix-command-line-too-long.

Excellent. Let's see if I can get reviews on the other one so it can land, then this PR can use just that stuff.

Thanks again!

@BillyONeal
Copy link
Member

People aren't comfortable attaching my more complete rework to this higher priority bugfix so we're going to merge this as is and we can switch to using a stdin pipe later. Thanks for your contribution!

@BillyONeal BillyONeal merged commit 5d96408 into microsoft:main Aug 1, 2023
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Aug 1, 2023
@klalumiere klalumiere deleted the commandLineTooLong branch August 2, 2023 02:28
BillyONeal added a commit that referenced this pull request Aug 12, 2023
Also uses the new infrastructure to avoid needing a temporary file in:
  [dependencygraph] Fix "command line too long" when sending to GitHub's dependency-graph API #1144
from @klalumiere .

Co-authored-by: Robert Schumacher <[email protected]>
Co-authored-by: Kevin Lalumiere <[email protected]>
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.

3 participants