Skip to content

Conversation

@jcgraybill
Copy link
Contributor

Since Github has deprecated basic authentication, change the gist-log command to prompt the user to create a Github personal access token any time HOMEBREW_GITHUB_API_TOKEN isn't found in the environment, rather than attempting to fall back to basic auth and failing. This should make it clearer to a user how to successfully post a build log to Github gists.

With basic auth functionality no longer available, several lines of code that had enabled it can also be removed - doing so.

Tested on Tiger PowerPC, successfully created these gists:

I'm also currently testing with the --new-issue flag, but wow does it take a long time for Ruby on one of these old Macintoshes to process the response body from a command that created the gist: I suspect it's trying to JSON parse the entire contents of the logs or something. The only thing it seems to use from the response body is the URL of the gist it just created, so a good optimization might be to do something more efficient than Utils::JSON.load get_body(response).

@jcgraybill
Copy link
Contributor Author

Okay, I've confirmed now that issue creation still works (see the issue referenced above.)

@jcgraybill
Copy link
Contributor Author

And indeed, the Github API response to create_gist includes the contents of each of the files (https://docs.github.com/en/rest/gists/gists?apiVersion=2022-11-28#create-a-gist). So gist-logs.rb reads back all the logfiles into memory and UTF-8 encodes them:

def get_body(response)
if !response.body.respond_to?(:force_encoding)
response.body
elsif response["Content-Type"].downcase == "application/json; charset=utf-8"
response.body.dup.force_encoding(Encoding::UTF_8)
else
response.body.encode(Encoding::UTF_8, :undef => :replace)
end
end
and then interprets the response as JSON:
Utils::JSON.load get_body(response)
and then extracts the "html_url" field
post("/gists", "public" => true, "files" => files)["html_url"]
so that it can pass it to new_issue() or print it on the command line.
url = new_issue(f.tap, "#{f.name} failed to build on #{MACOS_FULL_VERSION}", url, auth)

While this is all semantically correct, it took nearly an hour to run on a PowerPC Mac Mini. A much hackier but vastly quicker way to get the same string could be to grab it with a regular expression on the response body, e.g. /"html_url": "([^\"]+)"/.match(response.body)[1] which in my testing allowed the brew gist-logs command to complete in 38 seconds on the same PowerPC Mac Mini, and successfully created this issue/gist #1385.

If you don't think that's too much of a hack, I could amend this PR with that change, or would be glad to look into other optimization ideas.

@mistydemeo
Copy link
Owner

Nice work here! Thanks so much for this!

If you don't think that's too much of a hack, I could amend this PR with that change, or would be glad to look into other optimization ideas.

When we're dealing with old computers, nothing's too much of a hack - sometimes you have to do something a little weird in the name of performance. I'm fine with it if you think it's the fastest approach.

@jcgraybill
Copy link
Contributor Author

I've been testing this change against different packages for the past few days, and I think, on balance, it's a reasonable optimization. It's been reliable and quick. I went ahead and amended the PR with this commit: 15879e7. I'm continuing to run tests and let it create issues in my fork, and they're continuing to all look good.

Copy link
Owner

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks for the change and for testing.

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