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

🔧 Check for mac updates only #496

Merged
merged 2 commits into from
Feb 17, 2024
Merged

🔧 Check for mac updates only #496

merged 2 commits into from
Feb 17, 2024

Conversation

WangEdward
Copy link
Contributor

@WangEdward WangEdward commented May 31, 2023

Attempt to fix #491, #336 (and some other dup issue)

This idea is from https://github.com/mangerlahn/Latest(Thanks!). They also added "macSoftware" param, which I believe is unnecessary for us as it is not possible to upgrade an iOS-only app from command-line. (Not 100% sure, if that causes problem, we can add the param in the future)

@antifuchs
Copy link

Just tested this manually, using curl 'https://itunes.apple.com/lookup?id=1508732804&entity=desktopSoftware' | jq '.results[0].version' (Soulver 3 is one of the apps that have a different version on the desktop and on iOS), and that returns the correct version number. I think this might finally solve the issue of phantom-outdated versions for real!

@rmehner
Copy link

rmehner commented Jul 21, 2023

Can confirm that this fixes things for me as well. Thank you @WangEdward!

@iamrecursion
Copy link

Glad to see a fix is in the works! I ran into this one recently.

@rmehner
Copy link

rmehner commented Jul 31, 2023

@phatblat sorry for pinging you directly, but since the last commits all have been made / merged by you, I figured you might be the current maintainer of this.

What does it take to get this PR over the finish line? Happy to help with anything you need. Thank you!

@pawisoon
Copy link

Can we get this merged please? @phatblat

@nicerloop
Copy link

A custom formula for homebrew is available as nicerloop/nicerloop/mas in https://github.com/nicerloop/homebrew-nicerloop
Feel free to test.

@WafaAmr
Copy link

WafaAmr commented Oct 26, 2023

Can we get this merged please? @phatblat

Copy link
Contributor

@rgoldberg rgoldberg left a comment

Choose a reason for hiding this comment

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

Looks good

@rgoldberg
Copy link
Contributor

I haven't dealt with GitHub workflows before.

The workflow seems stuck at:

Build, Test, and Lint Expected — Waiting for status to be reported

Must we "Bump actions/checkout from 3 to 4" via #500 to unstick the workflow?

I've seen suggestions to amend the PR's commit, but I'd prefer to avoid messing with the git commits if It can / must be fixed via the GibHub interface / configuration.

@rmehner
Copy link

rmehner commented Nov 7, 2023

Hi @rgoldberg,

thanks for having a look, I'm excited to see this finally going forward again :)

Build, Test, and Lint Expected — Waiting for status to be reported
Must we "Bump actions/checkout from 3 to 4" via #500 to unstick the workflow?

This might solve the issue, if you rebase this branch then. If I'm not mistaken, it's merely an issue if retriggering the workflow that got stuck for whatever reason. The easiest way to do this is to add another commit to the PR / rebase the branch and then push again.

@WangEdward

This comment was marked as resolved.

@GreyTeardrop
Copy link

According to GitHub docs, first-time contributors creating PRs from their pubic forks need maintainer approval before CI kicks in: https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks.

@rgoldberg
Copy link
Contributor

According to GitHub docs, first-time contributors creating PRs from their pubic forks need maintainer approval before CI kicks in: https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks.

@GreyTeardrop Thanks for the info. I don't see "Workflow(s) awaiting approval" anywhere, so I assume that I'm not authorized to approve workflows. I'm only a Member; the only Maintainer is @phatblat.

@rgoldberg
Copy link
Contributor

@WangEdward @rmehner @GreyTeardrop

Thanks for the suggestions. I think that either @WangEdward should follow @rmehner's suggestion (potentially doing what's suggested at https://stackoverflow.com/a/58190576/2030088) or that I should try to make my own PR duplicating @WangEdward's one. I might have more success because I've already contributed & am a project Member, so I might be authorized to start workflows from my PRs, and because I'll use a branch other than main.

@WangEdward You choose what you want to do, as it's your PR. If I don't hear back from you in 48 hours or so, I'll just make my own PR, in which I'll credit you, just to keep everything moving.

Thanks for the fix.

@WangEdward
Copy link
Contributor Author

I force pushed it, and I can see the workflows are now in a state of awaiting approval. However, I don’t think there is any self-hosted CI machine attached to this repo, and the workflow will fail just like https://github.com/mas-cli/mas/actions/runs/6073669475 .

# https://docs.github.com/en/actions/hosting-your-own-runners/using-self-hosted-runners-in-a-workflow
# https://github.com/mas-cli/mas/settings/actions/runners
runs-on: [self-hosted, macOS]

@rgoldberg
Copy link
Contributor

@MikeMcQuaid We're working on getting this merged & released. The regular maintainers are MIA, but I'm a project member (with no knowledge of the project's workflows, release procedures, etc.). I'll let you know when it's released so you'll be able to revert Homebrew/homebrew-bundle#1231.

@rgoldberg
Copy link
Contributor

@WangEdward Do you mind if I make my own PR using a non-main branch to see if it works better for me?

@WangEdward
Copy link
Contributor Author

WangEdward commented Nov 7, 2023

@WangEdward Do you mind if I make my own PR using a non-main branch to see if it works better for me?

Please feel free to do so. I hope that will work.

@MikeMcQuaid
Copy link

@rgoldberg thanks for the heads up. keep me posted!

@rgoldberg
Copy link
Contributor

rgoldberg commented Nov 7, 2023

I don't see any runners listed on https://github.com/mas-cli/mas/actions/runners?tab=self-hosted.

I have no idea why self-hosted was specified without any currently extant runners. Maybe they previously existed but are now gone.

If anyone knows GitHub workflows, actions, etc., help will be appreciated.

@phatblat Any ideas? Is/was @ldgmini somehow related to the self-hosted runner? What does @masclibot do?

@antifuchs
Copy link

antifuchs commented Nov 7, 2023

If this repo works like most repos on github, running the CI jobs will be blocked until a repository admin / owner approves running the workflow. That's independent of any base branch to the PR.

This behavior is supposed to prevent exfiltration of github actions secrets (correction: apparently it's to prevent arbitrary "extra" code like crypto miners from being run as part of a test suite? There's lots of reasons not to let The Internet submit code that gets automatically started I guess). But as we have all witnessed, it results in an unknown in-between state where nobody knows what's wrong if the repo owner isn't around to press the button.

@GreyTeardrop
Copy link

@rgoldberg I was able to set up a self-hosted runner on my laptop with scripts from https://github.com/mas-cli/m1-github-actions-runner, attach it to my fork, recreate this PR in that fork, and run some CI for it. The CI run failed, probably because my laptop is on Sonoma and something has to be tweaked for it to properly build on Sonoma (oddly enough, the step that failed is "Lint", not "Build").

@rgoldberg
Copy link
Contributor

rgoldberg commented Nov 7, 2023

@GreyTeardrop I was busy until now. Thanks again for your efforts.

I don't have an Apple Silicon Mac (I'm still running an Intel Mac), so I might not be able to setup a working runner.

Do you have any local logs for the CI run on your computer? Specifically for the Lint step?

It's possible that the problem is that swiftlint wasn't installed:

The Brewfile (which I assume is used by script/bootstrap) doesn't install swiftlint (which is run in script/lint), but it isn't included in macOS 12.7.1 (I don't know about macOS 13+). A comment mentions that it's "Already installed on GitHub Actions runner", but it's likely not already installed on your machine. If you uncomment brew "swiftlint", maybe the Lint step will succeed.

If that doesn't work, you can probably just make script/lint a no-op by deleting everything in the file. If that works, I think it would be fine for us to "skip" the Lint step for now since this PR's change is so small, and since everyone is waiting on it.

The other commands used in script/lint (besides swift and git, but they should both be included in macOS in /usr/bin/) are listed in Brewfile. swift & shellcheck are run, but are not listed in the linter list. I don't think that shellcheck is included with macOS (I didn't find it in macOS 12.7.1), but it's in Brewfile, so it should still be available to script/lint.

Also note that Makefile runs swiftenv, which must be installed from the kylef/formulae tap. I don't know why that would affect the Lint step, but not others, so I assume that's not the problem.

I saw some announcements about GitHub now providing Apple Silicon extra large runners, but I doubt those would be available for free usage. I haven't seen anything about smaller runners. If anyone knows if GitHub provides any free Apple Silicon runners, please let everyone know. I can probably look into that tomorrow.

@rmehner
Copy link

rmehner commented Nov 8, 2023

@rgoldberg if you need a macos runner, you might be able to use the one https://tart.run provides. Example on how to do that is right on their landing page (the diff).

@rgoldberg
Copy link
Contributor

@rgoldberg if you need a macos runner, you might be able to use the one https://tart.run provides. Example on how to do that is right on their landing page (the diff).

@rmehner thanks for the info, but I don't have Apple Silicon; I'm still using Intel. It might useful for someone else who has Apple Silicon and who is willing to provide a runner.

@rmehner
Copy link

rmehner commented Nov 8, 2023

@rgoldberg tart provides the runner, so it doesn't run on your local machine.

@rgoldberg
Copy link
Contributor

@rmehner It reads to me like Tart is virtualization software while Cirrus Runners is a service using Tart that provides GitHub Runners. While usage of Tart is free for fewer than 100 cores, Cirrus Runners cost $150 / month each. Maybe I'm mistaken, but that's what it reads like to me. Do you have any link to any information that shows otherwise?

@GreyTeardrop
Copy link

Do you have any local logs for the CI run on your computer? Specifically for the Lint step?

It doesn't seem to write any logs locally, apparently everything meaningful is streamed to the Actions console:

> ./start.sh

√ Connected to GitHub

Current runner version: '2.311.0'
2023-11-07 21:41:17Z: Listening for Jobs
2023-11-07 21:41:40Z: Running job: Build, Test, and Lint
2023-11-07 21:47:07Z: Job Build, Test, and Lint completed with result: Failed
^CExiting...
Runner listener exit with 0 return code, stop the service, no retry needed.
Exiting runner...

It's possible that the problem is that swiftlint wasn't installed:

Based on what I see in the script, it should install swiftlint automatically if one isn't present.

I'll see what else can be done a bit later.

@rgoldberg
Copy link
Contributor

rgoldberg commented Nov 8, 2023

Probably simplest to just delete everything from, or do whatever else necessary to, script/lint to make it a no-op.

Based on what I see in the script, it should install swiftlint automatically if one isn't present.

Thanks. I didn't continue reading script/bootstrap after the call to brew bundle …, nor did I search for swiftlint after seeing the commented line in Brewfile. It's odd that swiftlint is handled differently instead of through Brewfile. You would think that brew bundle could handle a formula that is already installed…

@phatblat phatblat self-assigned this Nov 25, 2023
@phatblat
Copy link
Member

Sorry for the delay everyone. I've been swamped with work lately and haven't had time for open source contributions. I'm working on testing this now.

@GreyTeardrop
Copy link

GitHub made M1 runners available for free for open-source projects: https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/ 🎉

@rgoldberg
Copy link
Contributor

@GreyTeardrop See #508

@phatblat phatblat merged commit 85a31ed into mas-cli:main Feb 17, 2024
3 checks passed
@phatblat
Copy link
Member

phatblat commented Feb 18, 2024

@rgoldberg if you need a macos runner, you might be able to use the one https://tart.run provides. Example on how to do that is right on their landing page (the diff).

@rmehner I love tart! Cirrus Labs' tart runners cost $150 per month. #509 switched back to GitHub-hosted runners, free for open source projects like this one.

@MikeMcQuaid MikeMcQuaid mentioned this pull request Feb 29, 2024
@astrostl
Copy link

astrostl commented Jun 8, 2024

If I understand correctly, a fix for the long-standing fake upgrade problem already exists and is merged, but not released. What is standing in the way of that? Thanks!

@tonyarnold
Copy link
Contributor

@phatblat can I help out with administrative load here? I'm happy to do PR review, releases, whatever helps to keep things moving while you're busy.

@phatblat
Copy link
Member

@tonyarnold That would help immensely. I'm traveling in Japan for another couple weeks.

I worked up a release action in #510 but had trouble testing it with a pre release. #506 is the next action item but that was hard to delegate to someone without homebrew experience.

@tonyarnold
Copy link
Contributor

No problems, I'll get a brew allowlist into place in the local tap, which should help with the workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet