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

MacOS; export PATH variables for includes and libraries. #1420

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

simonjwright
Copy link
Contributor

Homebrew and MacPorts install include files and libraries in places where GCC won't look by default.

GCC will use these environment variables if set:

C_INCLUDE_PATH for C includes
CPLUS_INCLUDE_PATH for C++ includes
LIBRARY_PATH for libraries

Both of the distribution managers place (symbolic links to) include files in ${top_level}/include and libraries in ${top_level}/lib.

For Homebrew on Intel silicon, top_level is normally /usr/local.
For Homebrew on Apple silicon, top_level is normally /opt/homebrew.
For MacPorts, top_level is normally /opt/local

  • src/alire/alire-platforms-current.ads (Load_Environment): add note on macOS use.
  • src/alire/os_macos/alire-platforms-current__macos.adb (context): added Alire.Environment (was limited), Ada.Directories. (Brew_Access): new. (Homebrew_Present): if Brew_Access is not null. (Detected_Distribution): made into an expression function. (Containing_Containing_Dir): new, used in Distribution_Root. (Distribution_Root): reworked. (Load_Environment): if either distribution is present, arrange to export the environment variables to suit.

Homebrew and MacPorts install include files and libraries in places
where GCC won't look by default.

GCC will use these environment variables if set:

   C_INCLUDE_PATH     for C includes
   CPLUS_INCLUDE_PATH for C++ includes
   LIBRARY_PATH       for libraries

Both of the distribution managers place (symbolic links to) include
files in ${top_level}/include and libraries in ${top_level}/lib.

   For Homebrew on Intel silicon, top_level is normally /usr/local.
   For Homebrew on Apple silicon, top_level is normally /opt/homebrew.
   For MacPorts,                  top_level is normally /opt/local

  * src/alire/alire-platforms-current.ads (Load_Environment): add note
      on macOS use.
  * src/alire/os_macos/alire-platforms-current__macos.adb
    (context): added Alire.Environment (was limited), Ada.Directories.
    (Brew_Access): new.
    (Homebrew_Present): if Brew_Access is not null.
    (Detected_Distribution): made into an expression function.
    (Containing_Containing_Dir): new, used in Distribution_Root.
    (Distribution_Root): reworked.
    (Load_Environment): if either distribution is present, arrange to
      export the environment variables to suit.
@simonjwright
Copy link
Contributor Author

The problem with the macOS tests is this difference:

-Ada -> Alire_Host_Distro: distro_unknown
+Ada -> Alire_Host_Distro: homebrew

The reason is that I changed the detection of Homebrew from looking for $HOMEBREW_PREFIX to checking whether brew is on the PATH, and Github CI loads Homebrew automatically (into /usr/local on Intel silicon, which is why we now detect it).

Sorry about that. What should I do?

@Fabien-Chouteau
Copy link
Member

Hi @simonjwright, the testsuite also has a distrib detection system:

if os.environ.get('HOMEBREW_PREFIX'):

It should be adjusted to match the Alire one.

  * testsuite/drivers/helpers.py (distribution): if on macOS, check whether
      the distribution management tool is on the PATH. We used to check for
      the environment variable HOMEBREW_PREFIX, but users don't have to
      arrange for this to be set in order to run Homebrew.

      First, if 'brew' is found, the distribution is Homebrew.
      If not and 'port' is found, the distribution is MacPorts.
      Otherwise, the distribution is unknown.
  * .github/workflows/ci-macos.yml (Run test script): remove the second
      call, which set up HOMEBREW_PREFIX (now no longer used by alr),
      and remove the note '(without Homebrew)' in the first.
@simonjwright
Copy link
Contributor Author

Can this be reviewed, please?

Comment on lines -36 to -50
- name: Run test script (without Homebrew)
- name: Run test script
run: scripts/ci-github.sh
shell: bash
env:
BRANCH: ${{ github.base_ref }}
INDEX: ""

- name: Run test script (with Homebrew)
run: |
eval $(brew shellenv)
scripts/ci-github.sh
shell: bash
env:
BRANCH: ${{ github.base_ref }}
INDEX: ""
Copy link
Member

Choose a reason for hiding this comment

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

So I understand that now tests run by default without either hombrew or macports, unless we purposely activate one of those in the tests themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests now run with Homebrew (alr now checks whether brew is in the path, rather than whether HOMEBREW_PREFIX is set; the extra in the 'with Homebrew' part of the script was to set it).

I didn’t make a 'with MacPorts' version of the tests because CI automatically installs Homebrew; I’m sure we could install MacPorts (but needs sudo), but not sure about uninstalling Homebrew. For my purposes, I could just sudo rm $(which brew) - would that work? would you like another PR to do that?

I could reverse the test order so that, if both are present, MacPorts takes precedence, then wouldn’t need to rm brew. Grr, but if needs must ...

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for the explanation. I'm going to merge this one for now. I'm not sure how critical is to test the three situations; it might be overkill (for example we don't test Windows without msys2 IIRC).

@mosteo
Copy link
Member

mosteo commented Aug 21, 2023

Sorry for the wait, I had a concatenation of busy days.

@mosteo mosteo merged commit 239ac4e into alire-project:master Aug 21, 2023
12 checks passed
mosteo added a commit to mosteo/alire that referenced this pull request Aug 21, 2023
* Debug when folder deletion fails

* Trivial safeguard in `Force_Delete` (alire-project#1422)

* Refactor Alire.Shared into Alire.Toolchains (alire-project#1423)

* Use build profile in build hash (alire-project#1425)

* Compute hash using build profile

* Fix problem with multiple hashes in one run

Rooted in that default build profiles were used first and later the actual
profiles caused different folders to be used, in turn causing errors with the
conflicting CRATE_ALIRE_PREFIX variables.

* Write hash inputs to build dirs

* Testsuite additions and fixes

* Self-review

* Update pins and dependencies

* MacOS; export PATH variables for includes and libraries. (alire-project#1420)

* MacOS; export PATH variables for includes and libraries.

Homebrew and MacPorts install include files and libraries in places
where GCC won't look by default.

GCC will use these environment variables if set:

   C_INCLUDE_PATH     for C includes
   CPLUS_INCLUDE_PATH for C++ includes
   LIBRARY_PATH       for libraries

Both of the distribution managers place (symbolic links to) include
files in ${top_level}/include and libraries in ${top_level}/lib.

   For Homebrew on Intel silicon, top_level is normally /usr/local.
   For Homebrew on Apple silicon, top_level is normally /opt/homebrew.
   For MacPorts,                  top_level is normally /opt/local

  * src/alire/alire-platforms-current.ads (Load_Environment): add note
      on macOS use.
  * src/alire/os_macos/alire-platforms-current__macos.adb
    (context): added Alire.Environment (was limited), Ada.Directories.
    (Brew_Access): new.
    (Homebrew_Present): if Brew_Access is not null.
    (Detected_Distribution): made into an expression function.
    (Containing_Containing_Dir): new, used in Distribution_Root.
    (Distribution_Root): reworked.
    (Load_Environment): if either distribution is present, arrange to
      export the environment variables to suit.

* Update testsuite to match new macOS distribution detection.

  * testsuite/drivers/helpers.py (distribution): if on macOS, check whether
      the distribution management tool is on the PATH. We used to check for
      the environment variable HOMEBREW_PREFIX, but users don't have to
      arrange for this to be set in order to run Homebrew.

      First, if 'brew' is found, the distribution is Homebrew.
      If not and 'port' is found, the distribution is MacPorts.
      Otherwise, the distribution is unknown.

* In the macOS CI workflow, run the test script once only.

  * .github/workflows/ci-macos.yml (Run test script): remove the second
      call, which set up HOMEBREW_PREFIX (now no longer used by alr),
      and remove the note '(without Homebrew)' in the first.

* Add Windows alternatives

* More comprehensive Windows switches

* Don't raise on deletion failure

---------

Co-authored-by: Simon Wright <[email protected]>
@simonjwright simonjwright deleted the mac-build-paths branch November 9, 2023 16:30
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.

3 participants