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.

Sign up for GitHub

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

Adapt distribution of etc/profile.d/mamba.sh #283

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adapt distribution of etc/profile.d/mamba.sh #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b81f865
Adapt distribution of `etc/profile.d/mamba.sh`
jjerphan Jan 3, 2025
2045b78
build: Only for Unix systems
jjerphan Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions recipe/0001-build-Generate-and-install-etc-profile.d-mamba.sh-fo.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
From 2adbb72410fa6e481150a4384d556ebfbe82c28e Mon Sep 17 00:00:00 2001
From: Julien Jerphanion <[email protected]>
Date: Fri, 3 Jan 2025 11:36:14 +0100
Subject: [PATCH] build: Generate and install `etc/profile.d/mamba.sh` for
mamba only

Signed-off-by: Julien Jerphanion <[email protected]>
---
mamba_package/CMakeLists.txt | 10 ---------
micromamba/CMakeLists.txt | 21 ++++++++++++++++++-
.../etc/profile.d/mamba.sh.in | 0
3 files changed, 20 insertions(+), 11 deletions(-)
rename {mamba_package => micromamba}/etc/profile.d/mamba.sh.in (100%)

diff --git a/mamba_package/CMakeLists.txt b/mamba_package/CMakeLists.txt
index 23a23e95..cf47433a 100644
--- a/mamba_package/CMakeLists.txt
+++ b/mamba_package/CMakeLists.txt
@@ -38,16 +38,6 @@ target_link_libraries(mamba-package PRIVATE mamba::libmamba)

set_target_properties(mamba-package PROPERTIES CXX_STANDARD 17)

-configure_file(
- "${CMAKE_CURRENT_SOURCE_DIR}/etc/profile.d/mamba.sh.in"
- "${CMAKE_CURRENT_BINARY_DIR}/etc/profile.d/mamba.sh"
-)
-
-install(
- FILES ${CMAKE_CURRENT_BINARY_DIR}/etc/profile.d/mamba.sh
- DESTINATION ${CMAKE_INSTALL_PREFIX}/etc/profile.d/
-)
-
install(
TARGETS mamba-package
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
diff --git a/micromamba/CMakeLists.txt b/micromamba/CMakeLists.txt
index a4f2f152..4d313644 100644
--- a/micromamba/CMakeLists.txt
+++ b/micromamba/CMakeLists.txt
@@ -105,4 +105,23 @@ endif()
# Installation
# ============

-install(TARGETS ${mambaexe_targets})
+# The script are only needed for mamba and not for micromamba
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that I'm only claiming is that the mamba.sh/mamba.bat activation script should not be included with libmamba.

Probably your claim is correct, but it caught my eye. (It's been a long time since I've looked at the activation scripts for micromamba, and indeed the latest micromamba package doesn't seem to include any activation scripts.)

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference, this change was added based on this test in micromamba-feedstock independently from your remark.

+
+if(BUILD_SHARED)
+ configure_file(
+ "${CMAKE_CURRENT_SOURCE_DIR}/etc/profile.d/mamba.sh.in"
+ "${CMAKE_CURRENT_BINARY_DIR}/etc/profile.d/mamba.sh"
+ )
+
+ install(
+ FILES ${CMAKE_CURRENT_BINARY_DIR}/etc/profile.d/mamba.sh
+ DESTINATION ${CMAKE_INSTALL_PREFIX}/etc/profile.d/
+ )
+endif()
+
+install(
+ TARGETS ${mambaexe_targets}
+ RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
+ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
+)
diff --git a/mamba_package/etc/profile.d/mamba.sh.in b/micromamba/etc/profile.d/mamba.sh.in
similarity index 100%
rename from mamba_package/etc/profile.d/mamba.sh.in
rename to micromamba/etc/profile.d/mamba.sh.in
--
2.47.1

32 changes: 32 additions & 0 deletions recipe/0002-build-Only-for-Unix-systems.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
From 33713ee0b44fea3d4ee78ab40d403bebb0e75daf Mon Sep 17 00:00:00 2001
From: Julien Jerphanion <[email protected]>
Date: Mon, 6 Jan 2025 13:17:03 +0100
Subject: [PATCH] build: Only for Unix systems

Signed-off-by: Julien Jerphanion <[email protected]>

Co-authored-by: Klaim <[email protected]>
---
micromamba/CMakeLists.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/micromamba/CMakeLists.txt b/micromamba/CMakeLists.txt
index 4d313644..a75d436d 100644
--- a/micromamba/CMakeLists.txt
+++ b/micromamba/CMakeLists.txt
@@ -105,9 +105,9 @@ endif()
# Installation
# ============

-# The script are only needed for mamba and not for micromamba
-
-if(BUILD_SHARED)
+# This profile script is only needed for mamba on Unix systems.
+if(BUILD_SHARED AND UNIX)
+ message(STATUS "Generating profile script for mamba (i.e. `etc/profile.d/mamba.sh`)")
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/etc/profile.d/mamba.sh.in"
"${CMAKE_CURRENT_BINARY_DIR}/etc/profile.d/mamba.sh"
--
2.47.1

10 changes: 7 additions & 3 deletions recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ package:
source:
url: https://github.com/mamba-org/mamba/archive/refs/tags/{{ release }}.tar.gz
sha256: 4960fc5aa617310fb45e309a1bcfeae74085d70782443b1d2882de4400ac6a48
patches:
- 0001-build-Generate-and-install-etc-profile.d-mamba.sh-fo.patch
- 0002-build-Only-for-Unix-systems.patch

build:
number: 1
number: 2

outputs:
- name: libmamba
Expand Down Expand Up @@ -63,11 +66,13 @@ outputs:

test:
commands:
- test ! -f "${PREFIX}/etc/profile.d/mamba.sh" # [unix]
- test -d ${PREFIX}/include/mamba # [unix]
- test -f ${PREFIX}/include/mamba/version.hpp # [unix]
- test -f ${PREFIX}/lib/cmake/libmamba/libmambaConfig.cmake # [unix]
- test -f ${PREFIX}/lib/cmake/libmamba/libmambaConfigVersion.cmake # [unix]
- test -e ${PREFIX}/lib/libmamba${SHLIB_EXT} # [unix]
- if exist %LIBRARY_PREFIX%\etc\profile.d\mamba.sh (exit 1) # [win]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be .bat on Windows. Also suggesting to use the identical path from the mamba section below.

Suggested change
- if exist %LIBRARY_PREFIX%\etc\profile.d\mamba.sh (exit 1) # [win]
- if exist %PREFIX%\condabin\mamba.bat (exit 1) # [win]

Copy link
Member Author

@jjerphan jjerphan Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This tests that the file is also not present on Windows (this can be misread at first).

I think we could move this check at the top with the one for [unix] with a comment giving the motivation and to remove the confusion.

- if not exist %LIBRARY_PREFIX%\include\mamba\version.hpp (exit 1) # [win]
- if not exist %LIBRARY_PREFIX%\lib\cmake\libmamba\libmambaConfig.cmake (exit 1) # [win]
- if not exist %LIBRARY_PREFIX%\lib\cmake\libmamba\libmambaConfigVersion.cmake (exit 1) # [win]
Expand All @@ -76,8 +81,7 @@ outputs:
{% set libmamba_version_split = libmamba_version.split('.') %}
- cat $PREFIX/include/mamba/version.hpp | grep "LIBMAMBA_VERSION_MAJOR {{ libmamba_version_split[0] }}" # [unix]
- cat $PREFIX/include/mamba/version.hpp | grep "LIBMAMBA_VERSION_MINOR {{ libmamba_version_split[1] }}" # [unix]
# TODO: uncomment for release 2.0.0
#- cat $PREFIX/include/mamba/version.hpp | grep "LIBMAMBA_VERSION_PATCH {{ libmamba_version_split[2] }}" # [unix]
- cat $PREFIX/include/mamba/version.hpp | grep "LIBMAMBA_VERSION_PATCH {{ libmamba_version_split[2] }}" # [unix]

- name: libmambapy
version: {{ libmambapy_version }}
Expand Down