search

TEIC / TEI

The Text Encoding Initiative Guidelines
https://www.tei-c.org
Other
279 stars 88 forks source link

Fixing permissions in actions (should fix #2399) #2401

Closed joeytakeda closed 1 year ago

joeytakeda commented 1 year ago

Note that I haven't added the permissions step to generate-spec-lists.yml, as I don't think it will be necessary—however, I haven't been able to figure out a way to test for the dubious permissions error locally (or at least reliably), since I can't replicate the error (running the steps in a Docker container doesn't yield the error and neither does running the action using act).

ebeshero commented 1 year ago

@joeytakeda I merged this branch into my failing branch, and indeed, it worked so that the tests could complete running: https://github.com/TEIC/TEI/actions/runs/4411611402/jobs/7730289074

So I think this is a helpful repair to merge in to dev--and should indeed repair what's broken from the most recent commit there. @hcayless or @raffazizzi any issues with my merging to dev as requested?

raffazizzi commented 1 year ago

@joeytakeda this looks fine to me and the checks pass.

I was a little concerned that the git config --global --add safe.directory /__w/TEI/TEI may fail if the directory name were to change in the future, but I tested adding a non-existent safe.directory locally and it doesn't seem to be an issue. If the directory name does change in the future, I figure the future council will have to update the path.

I would maybe remove the comment # Temporary fix to deal with #2399 before merging.

hcayless commented 1 year ago

Would git config --global --add safe.directory "$GITHUB_WORKSPACE" work instead? and avoid the hard-coded directory path?

joeytakeda commented 1 year ago

Thanks @hcayless @raffazizzi!

I was a little concerned that the git config --global --add safe.directory /__w/TEI/TEI may fail if the directory name were to change in the future, but I tested adding a non-existent safe.directory locally and it doesn't seem to be an issue. If the directory name does change in the future, I figure the future council will have to update the path.

Would git config --global --add safe.directory "$GITHUB_WORKSPACE" work instead? and avoid the hard-coded directory path?

Good point — I'll change that now.

I would maybe remove the comment # Temporary fix to deal with #2399 before merging.

I added that comment so we can flag it for removal at some point (since this is really a bug with the action/checkout@v3 that I would hope would be solved in a future iteration) — but it's not a very helpful comment, so I will adjust to try and make it a bit more useful