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

Bail on activation when CONDA_BUILD is set #1694

Merged
merged 7 commits into from
Jan 24, 2025

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Jan 23, 2025

Otherwise, the GDB symlinks will be included in packages built with libarrow as a host dependency.

Closes #1478

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Otherwise, the GDB symlinks will be included in packages built with libarrow as a host dependency.
@maresb maresb force-pushed the dont-activate-from-build branch from 39167f1 to 7b30757 Compare January 23, 2025 17:41
@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

For recipe/meta.yaml:

  • ℹ️ The boost-cpp output has been superseded by libboost-devel (as of 1.82), which now comes with a run-export (on libboost) as well. In case you only needed the boost headers, you should use libboost-headers.
  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12934897643. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 23, 2025

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12957715354. Examine the logs at this URL for more detail.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Thank you very much! 🙏 🙂

# Skip activation if CONDA_BUILD environment variable is set.
# Otherwise, the symlinks will be included in packages built with libarrow as a host dependency.
# <https://github.com/prefix-dev/rattler-build/issues/979#issuecomment-2243070530>
if [ -n "$CONDA_BUILD" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Ah wait, CI is deep red, failing in

- test -f $PREFIX/share/gdb/auto-load/$PREFIX/lib/libarrow.so.{{ so_version }}-gdb.py # [linux]
- test -f $PREFIX/share/gdb/auto-load/$PREFIX/lib/libarrow.{{ so_version }}.dylib-gdb.py # [osx]

Is -n the right argument here?

@h-vetinari
Copy link
Member

h-vetinari commented Jan 24, 2025

It seems that CONDA_BUILD is still set during the test phase for some reason (so probably your implementation was correct on the first go!). I had been under the impression that this is not the case, but looking at the docs, perhaps I was wrong...? 🤔

In this case, just add something like

# need to unset this to test gdb-integration despite https://github.com/conda-forge/arrow-cpp-feedstock/issues/1478
- unset CONDA_BUILD  # [unix]

before

- test -f $PREFIX/share/gdb/auto-load/$PREFIX/lib/libarrow.so.{{ so_version }}-gdb.py # [linux]
- test -f $PREFIX/share/gdb/auto-load/$PREFIX/lib/libarrow.{{ so_version }}.dylib-gdb.py # [osx]

@maresb
Copy link
Contributor Author

maresb commented Jan 24, 2025

Indeed.

In this case, just add something like

# need to unset this to test gdb-integration despite https://github.com/conda-forge/arrow-cpp-feedstock/issues/1478
- unset CONDA_BUILD  # [unix]

I wouldn't expect that to have any effect post-initialization though.

I'm a bit stumped at the moment. (Or do you still think your suggestion might work?)

@h-vetinari
Copy link
Member

I wouldn't expect that to have any effect post-initialization though.

Ah, true!

@jaimergp @jakirkham @beckermr, would you have any idea whether it's intentional that CONDA_BUILD is set also during the test phase? I seemed to remember that that's not the case, but perhaps that was just my imagination?

@xhochy
Copy link
Member

xhochy commented Jan 24, 2025

You should be able to lookup the testing phase using "${CONDA_BUILD_STATE:-0}" = "TEST".

@maresb
Copy link
Contributor Author

maresb commented Jan 24, 2025

Thanks @xhochy, trying that now.

TODO: remove the debug commit... converting to draft

@maresb maresb marked this pull request as draft January 24, 2025 18:23
@h-vetinari
Copy link
Member

I took the liberty to clean up the debug line and add a guard against unset variables. Since the resulting changes

git diff d1919bbf00cd928e036d0987ab86d25fa1a7b119..197c2cb181665e0bc9059d654a1fdeadb076c045
diff --git a/recipe/activate.sh b/recipe/activate.sh
index bc796802..2ec4ad24 100755
--- a/recipe/activate.sh
+++ b/recipe/activate.sh
@@ -6,7 +6,7 @@

 # doesn't come with a deactivate script, because the symlink
 # is benign and doesn't need to be deleted.
-CF_LIBARROW_ACTIVATE_LOGGING=1
+
 _la_log() {
     if [ "${CF_LIBARROW_ACTIVATE_LOGGING:-}" = "1" ]; then
         # The following loop is necessary to handle multi-line strings
@@ -21,9 +21,9 @@ _la_log() {
 # Skip activation if CONDA_BUILD environment variable is set.
 # (CONDA_BUILD is also set in the test stage, and we don't want to skip there.)
 # Otherwise, the symlinks will be included in packages built with libarrow as a host dependency.
-# <https://github.com/prefix-dev/rattler-build/issues/979#issuecomment-2243070530>
-if [ -n "$CONDA_BUILD" ] && [ "$CONDA_BUILD_STATE" != "TEST" ]; then
-    _la_log "CONDA_BUILD is set to $CONDA_BUILD and CONDA_BUILD_STATE is $CONDA_BUILD_STATE, skipping libarrow activation."
+# see https://github.com/conda-forge/arrow-cpp-feedstock/issues/1478
+if [ -n "$CONDA_BUILD" ] && [ "${CONDA_BUILD_STATE:-0}" != "TEST" ]; then
+    _la_log "CONDA_BUILD is set to $CONDA_BUILD (and CONDA_BUILD_STATE != \"TEST\"), skipping libarrow activation."
     return 0
 fi

do not change anything functionally, I'm further taking the liberty of merging this. Hope that's OK. :)

@h-vetinari h-vetinari marked this pull request as ready for review January 24, 2025 21:17
@h-vetinari h-vetinari merged commit 1eef11c into conda-forge:main Jan 24, 2025
2 of 13 checks passed
@h-vetinari
Copy link
Member

Thanks a lot for the PR, and thanks for the tip @xhochy! 🥳

@maresb maresb deleted the dont-activate-from-build branch January 24, 2025 21:50
@maresb
Copy link
Contributor Author

maresb commented Jan 24, 2025

Of course, thanks so much @h-vetinari!

h-vetinari pushed a commit that referenced this pull request Jan 24, 2025
Use CONDA_BUILD_STATE to prevent bailing on test
h-vetinari pushed a commit that referenced this pull request Jan 24, 2025
Use CONDA_BUILD_STATE to prevent bailing on test
h-vetinari pushed a commit that referenced this pull request Jan 24, 2025
Use CONDA_BUILD_STATE to prevent bailing on test
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.

Activation script creates new files?
4 participants