Implement Homebrew as the system deployer for macOS.#1185
Merged
Conversation
* src/alire/alire-origins-deployers-system-homebrew.adb: new.
* src/alire/alire-origins-deployers-system-homebrew.ads: new.
* src/alire/alire-origins-deployers-system.adb (Platform_Deployer):
When the platform distro manager is Homebrew, return Homebrew.Deployer.
* src/alire/alire-platforms.ads (Distributions): include macOS.
(Package_Managers): add Homebrew.
(Distro_Manager): for macOS, use Homebrew.
* src/alire/alire-utils-tools.adb (System_Package_For_Tool): treat MacOS
like other distributions.
* src/alire/os_macos/alire-platforms-current__macos.adb
(Detected_Distribution): return Platforms.MacOS.
* testsuite/drivers/helpers.py (on_macos()): new.
(distribution()): return 'MACOS' if on_macos() is True.
; Squashed commit of the following:
; commit 0110299ceb66f58cd4793fd78a69fc5b100b4ccf
; Author: Simon Wright <simon@pushface.org>
; Date: Fri Sep 9 17:45:04 2022 +0100
; Restore test to correct sense.
; commit d396b6b54a73bbbfd59071539bf6268bccce3cf0
; Author: Simon Wright <simon@pushface.org>
; Date: Fri Sep 9 17:34:54 2022 +0100
; Replace assertions in Homebrew deployer with exceptions
; commit d7603110b9543bdaac9516fa844599e87fac70aa
; Author: Simon Wright <simon@pushface.org>
; Date: Mon Sep 5 14:14:03 2022 +0100
; Recognise macOS as a distribution.
; * testsuite/drivers/helpers.py (on_macos()): new.
; (distribution()): check on_macos(). Return 'MACOS' if true.
; commit 8703acfeb8fdf336d862cefc16706eac5d3f0758
; Author: Simon Wright <simon@pushface.org>
; Date: Sun Sep 4 17:33:13 2022 +0100
; Use 'brew info' to get available, installed versions.
; commit 8b22b904e19d64fecb89233e6527c2faf655d982
; Author: Simon Wright <simon@pushface.org>
; Date: Wed Aug 31 15:54:51 2022 +0100
; Continuing.
; commit a9821ed9a605e30b5699a49df6fffd11224ab931
; Author: Simon Wright <simon@pushface.org>
; Date: Mon Aug 22 17:09:48 2022 +0100
; First changes for macOS Homebrew.
Fabien-Chouteau
requested changes
Sep 12, 2022
Fabien-Chouteau
left a comment
Member
There was a problem hiding this comment.
Awesome @simonjwright !
I have a few comments, the main one being about the MacOS vs Homebrew distrib.
Thanks for your contribution,
|
|
||
| begin | ||
| Trace.Debug ("detect? " & This.Base.Package_Name); | ||
| if Homebrew_Present then |
Member
There was a problem hiding this comment.
Detection of the "distrib" should be done in the platform specific code. There's no need to do it here as far as I know.
|
|
||
| function Detected_Distribution return Platforms.Distributions is | ||
| (Platforms.Distro_Unknown); | ||
| (Platforms.MacOS); |
Member
There was a problem hiding this comment.
This is where you detect the HOMEBREW_PREFIX env variable and return Platforms.Homebew if present, Platforms.Distro_Unknown otherwise.
* .github/workflows/ci-macos.yml (Run test script): renamed to
"Run test script (without Homebrew)".
(Run test script (with Homebrew)): added 'eval $(brew shellenv)' before
running the test script, so that HOMEBREW_PREFIX gets defined.
* src/alire/alire-origins-deployers-system-homebrew.adb
(Homebrew_Prefix, Homebrew_Present): removed.
(Get_Info): named the declare block. Removed commented-out exception
handler.
(Already_Installed): use Get_Info's returned Installed_Version's length
to determine the result.
(Detect): Get_Version_From_String uses Semantic_Versioning.Parse.
Don't check for Homebrew_Present, it has to be or we wouldn't be
called.
* src/alire/alire-platforms.ads (Distributions): rename MacOS to Homebrew.
(Distro_Manager): likewise.
* src/alire/alire-utils-tools.adb (System_Package_For_Tool): MacOS (in
'case Distribution)' changed to Homebrew.
* src/alire/os_macos/alire-platforms-current__macos.adb
(Homebrew_Prefix): new, gets the environment variable "HOMEBREW_PREFIX".
(Homebrew_Present): new, true if the environment variable is present.
(Detected_Distribution): checks Homebrew_Present, returns Homebrew
or Distro_Unknown accordingly.
(Distribution_Root): returns Homebrew_Prefix if present, otherwise "/".
* testsuite/drivers/helpers.py (distribution()): if on macOS, return
HOMEBREW if HOMEBREW_PREFIX is found, DISTRO_UNKNOWN otherwise.
Implement Homebrew for MacOS: response to comments on PR#1185
Member
|
Looks good to me, outstanding effort, Simon. Since this PR has had quite some discussion, I'm going to wait for @Fabien-Chouteau feedback before merging. |
Fabien-Chouteau
requested changes
Sep 22, 2022
* src/alire/alire-origins-deployers-system-homebrew.adb: removed
the innefective pragma Warnings (Off).
Fabien-Chouteau
approved these changes
Sep 22, 2022
Member
|
Thanks @simonjwright, can you fix the conflicts with the OpenSUSE support that was just added? Sorry for that |
* src/alire/alire-origins-deployers-system.adb: added Homebrew option. * src/alire/alire-platforms.ads: likewise. * src/alire/alire-utils-tools.adb: likewise.
Contributor
Author
|
I did fix the conflicts, and hoped to see CI runs; maybe the reason why they didn’t happen is that Github thinks there are still conflicts? |
Member
|
GitHub says there's still one conflic: |
Contributor
Author
|
Actually, Github says there are 3 conflicts, but they are in fact the changes that I want to make. I don’t see how I can fix this ... Oh. Perhaps the problem is that I cherry-picked the openSuse commit from master? ... but that introduced conflicts which I fixed, and I don’t see any other trace of it than the commit (cb3ca3b) author ... Oh². I can make it happen here. Baffled for the time being |
Member
|
Let me take a stab at this |
Member
|
Sorry for stepping in in your fixing, Simon. Merged now! |
Contributor
Author
Brilliant! Thanks .. |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Issue #1172.
It's not clear to me how we could deal with MacPorts as an alternate deployer, if anyone wants to do the work (won't be me).
I'm not sure about
System_Package_For_Toolinsrc/alire/alire-utils-tools.adb; I've treated macOS like all the other distributions so as to get a build, but I don't understand how/when this is used.