Skip to content
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

Update macdown from 0.7.2 to 0.7.3 #75036

Closed

Conversation

ShivaHuang
Copy link
Contributor

After making all changes to the cask:

  • brew cask audit --download {{cask_file}} is error-free.
  • brew cask style --fix {{cask_file}} left no offenses.
  • The commit message includes the cask’s name and version.

@vitorgalvao
Copy link
Member

Can you install this version locally?

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Jan 3, 2020
@ShivaHuang
Copy link
Contributor Author

No, unfortunately.
I run

brew cask reinstall https://raw.githubusercontent.com/ShivaHuang/homebrew-cask/cask_repair_update-macdown/Casks/macdown.rb

and got

==> Downloading https://raw.githubusercontent.com/ShivaHuang/homebrew-cask/cask_repair_update-macdown/Casks/macdown.rb.
==> Downloading https://github.com/MacDownApp/macdown/releases/download/v0.7.2/MacDown.app.zip

The downloaded file's version doesn't match the one in spec, thus checksum failed.

Any idea what may cause this?

@ShivaHuang
Copy link
Contributor Author

I think the situation is the same as #66596. But currently there are no findings.

@vitorgalvao
Copy link
Member

That’s irrelevant. The issue here tripping CI is a permission denied problem.

@varenc
Copy link
Contributor

varenc commented Jan 14, 2020

The issue with this is the /MacDown.app/./Contents/_CodeSignature file. It has 000 permissions. (not readable/writeable by anyone). I believe those permissions are specified in the zip file and unzip which homebrew uses seems to preserve those permissions.

When unzipped with macOS's built-in "Archive Utility" that file gets .rwxr-xr-x permissions instead. It probably overrides the permissions included in the zip in cases like this?

Also, the _CodeSignature file is empty so probably isn't doing anything at all. There was no file at that path in the previous release. Though obscure, I think this is a problem with the MacDown release and something that they should fix. Unless homebrew wants to start fixing permissions in cases like this.

@vitorgalvao
Copy link
Member

I think this is a problem with the MacDown release and something that they should fix.

It is. Someone should contact them.

Unless homebrew wants to start fixing permissions in cases like this.

We prefer not to, but it is doable.

@varenc
Copy link
Contributor

varenc commented Jan 15, 2020

Interesting! set_permissions seems like exactly what we'd need.

If I made a PR for this cask utilizing that in the preflight to fix this issue, would it be accepted? (Assuming good code. Just checking that it wouldn't be categorically rejected)

I opened an issue with MacDown but haven't waited long enough to get a response yet. If they fix it we're good, but if they don't care about this though then a fix in the cask might handy.

(In my earlier comment, I was thinking more like homebrew casks could, though it shouldn't, generally look for really pathological permissions cases like this that'll always cause errors and then fix them generally. Sort of like how the macOS Archive Utility seems to override the permissions the .zip file specifies and just uses sensible default permissions instead. I don't think that's a good practice to get into though and glad to hear.)

@ShivaHuang
Copy link
Contributor Author

ShivaHuang commented Jan 15, 2020

I tried set_permissions with preflight as follows

preflight do
    set_permissions "#{staged_path}/MacDown.app/Contents/_CodeSignature", '0755'
end

postflight do
    set_permissions "#{appdir}/MacDown.app/Contents/_CodeSignature", '0000'
end

but no luck. It seems the processes is something like

  1. Download zip file
  2. unzip file in temp folder
  3. move unzipped files to staged_path
  4. preflight -> set_permissions

But the process breaks on stage 3, so preflight doesn't execute at all.

I think it's better to fix it with macdown.

@vitorgalvao
Copy link
Member

But the process breaks on stage 3, so preflight doesn't execute at all.

Ah, right.

There are more things we can try (I seem to remember broken DMGs we could find some workarounds for), but at this point I’d rather wait for a response from upstream.

@ArchdukeTim ArchdukeTim mentioned this pull request Jan 22, 2020
4 tasks
@jnozsc
Copy link
Contributor

jnozsc commented Feb 14, 2020

upstream ticket MacDownApp/macdown#1144

@FranklinYu
Copy link
Contributor

FranklinYu commented Feb 21, 2020

Please try updating the testing branch of MacDown. I applied a fix there.

If the testing branch is updated successfully, I would backport the fix to stable branch, and we can proceed this PR.

Sorry, we have some issue releasing in the test branch. Will report back when it’s ready.

@FranklinYu
Copy link
Contributor

Good news! We finally released 0.8.0d71 on https://macdown.uranusjr.com/history/testing/. Please try the binary there; we should have removed the troublesome code signature in the binary.

@vitorgalvao vitorgalvao removed the awaiting user reply Issue needs response from a user. label Feb 24, 2020
@lock lock bot added the outdated label Mar 27, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants