Fix the shebang in certain scripts #192

hmaarrfk · 2024-09-28T16:00:07Z

Closes: #191

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.

github-actions · 2024-09-28T16:01:26Z

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.

hmaarrfk · 2024-09-29T20:40:36Z

recipe/meta.yaml

+        - head -n 1 ${PREFIX}/bin/glib-mkenums | grep -E "(${PREFIX}|/usr/bin/env python)"      # [unix]
+        - head -n 1 ${PREFIX}/bin/glib-genmarshal | grep -E "(${PREFIX}|/usr/bin/env python)"   # [unix]
+        - head -n 1 ${PREFIX}/bin/gtester-report | grep -E "(${PREFIX}|/usr/bin/env python)"    # [unix]


So i don't really understand why or how this became /usr/bin/env instead of the ${PREFIX}...

maybe it is a detail with how noarch python works?

It's because it's supposed to point to the environment Python when installed instead of some strange prefix only used during build, but I don't know the internal mechanism Conda uses to replace it to /usr/bin/env python.

recipe/install.sh

ehfd · 2024-10-03T09:31:15Z

@hmaarrfk Any news? Downstream, the only solution is to pin, which is pretty dirty.

recipe/meta.yaml

hmaarrfk · 2024-10-03T16:51:05Z

I think this is a step in the right direction.

hmaarrfk · 2024-10-03T16:51:42Z

but honestly, i don't really want want to maintain the "g" software stack too much, so I want some of the maintainers to chime in with quick review before merging.

isuruf · 2024-10-03T16:53:38Z

recipe/meta.yaml

@@ -205,6 +216,12 @@ outputs:
        - test -f ${PREFIX}/lib/libglib-2.0${SHLIB_EXT}             # [unix]
        - glib-compile-resources --help
        - gobject-query --help
+        # I'm not sure why the PREFIX ends up becoming the /usr/bin/env python


Shebang's can't be longer than 80 lines, so conda fixes them for you.

ah i see. Thanks!

In this case, is the test correct, or should it be made stricter? (i.e. only testing for /usr/bin/env python)?

Hmm ... did you check whether these tests fail if you don't apply the shebang fix? I am pretty sure that if the shebang line in the source file had $BUILD_PREFIX in it, the test would indeed fail, because Conda's prefix-rewriting logic wouldn't be activated at all.

If that's the case, then I think the test is fine as-is. I believe that in all of our build environments, the package test is run inside a directory that's got a sufficiently long absolute path that the /usr/bin/env replacement will always be active. But if you build this recipe outside of the conda-forge framework, the test prefix could conceivably be something like /c/conda-bld/glib-123556890/_test_env where the rewriting would not kick in.

Isuru's diagnosis is 100% correct, I believe, so it might be good to update the comment and add a link to this pull request.

ok lets test the tests. I think they worked in the past. but i forget.

well my tests fail to actually catch the error, but these replacements are necessary:

+ head -n 1 /home/conda/feedstock_root/build_artifacts/glib-split_1728918802549/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/bin/glib-mkenums + grep -E '^ head -n 1 /home/conda/feedstock_root/build_artifacts/glib-split_1728918802549/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/bin/glib-genmarshal | grep -E ^' #!/home/conda/feedstock_root/build_artifacts/glib-split_1728918802549/_build_env/bin/python3.13 + head -n 1 /home/conda/feedstock_root/build_artifacts/glib-split_1728918802549/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/bin/gtester-report ++ pkg-config --cflags --libs glib-2.0

Oooohhhhh that's tricky — it looks like the YAML parse is yielding surprising results for those lines. If they were passed through to the shell verbatim, I think they would work as you had them before. It might be worth encasing the whole lines in double quotes to make the YAML-level subtlety more apparent.

edit: Actually they wouldn't have worked before due to the missing exclamation points. But I think the \# is only necessary for the YAML parser, not the shell or regex parser.

I couldn’t get the matching of the # to work. But matching it with anything helped test for the prefix.

Now let’s see if the “fix” works

recipe/install.sh

ehfd · 2024-10-13T12:26:47Z

@hmaarrfk @isuruf Any reviews or finalizations?

pkgw

A couple of small suggestions but I think this is pretty much ready.

pkgw · 2024-10-14T14:08:29Z

recipe/install.sh

+# These files are the only difference when running with a different setting of -Dintrospection
+if [[ "${CONDA_BUILD_CROSS_COMPILATION:-0}" == 1 ]]; then
+    cp -ap introspection/lib/girepository-1.0 $PREFIX/lib
+cp -ap introspection/share/gir-1.0 $PREFIX/share


Some wonky indentation here.

pkgw · 2024-10-14T14:16:59Z

recipe/meta.yaml

@@ -205,6 +216,12 @@ outputs:
        - test -f ${PREFIX}/lib/libglib-2.0${SHLIB_EXT}             # [unix]
        - glib-compile-resources --help
        - gobject-query --help
+        # I'm not sure why the PREFIX ends up becoming the /usr/bin/env python


Hmm ... did you check whether these tests fail if you don't apply the shebang fix? I am pretty sure that if the shebang line in the source file had $BUILD_PREFIX in it, the test would indeed fail, because Conda's prefix-rewriting logic wouldn't be activated at all.

If that's the case, then I think the test is fine as-is. I believe that in all of our build environments, the package test is run inside a directory that's got a sufficiently long absolute path that the /usr/bin/env replacement will always be active. But if you build this recipe outside of the conda-forge framework, the test prefix could conceivably be something like /c/conda-bld/glib-123556890/_test_env where the rewriting would not kick in.

Isuru's diagnosis is 100% correct, I believe, so it might be good to update the comment and add a link to this pull request.

recipe/meta.yaml

hmaarrfk · 2024-10-14T20:21:32Z

Well lmk if there is anything else. But also feel free to push cosmetic or functional changes

pkgw · 2024-10-14T20:27:55Z

Thanks for slogging through this one!

Try to fix the shebank in certain scripts

491f5f7

hmaarrfk requested review from ccordoba12, isuruf, jakirkham, ocefpaf, pkgw, scopatz, tschoonj and xhochy as code owners September 28, 2024 16:00

hmaarrfk added 2 commits September 28, 2024 12:21

Try again

1d3806b

Add debuggging outputs

1a0ea2e

pkgw changed the title ~~Try to fix the shebank in certain scripts~~ Try to fix the shebang in certain scripts Sep 29, 2024

Fix things but i don't understand why

43dbced

hmaarrfk commented Sep 29, 2024

View reviewed changes

Be even more strict

60890d9

hmaarrfk commented Sep 29, 2024

View reviewed changes

recipe/install.sh Outdated Show resolved Hide resolved

fix typo

16c013f

hmaarrfk commented Oct 3, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

hmaarrfk added 2 commits October 3, 2024 12:49

add a note

54243e8

fix typos

b1ee588

isuruf reviewed Oct 3, 2024

View reviewed changes

hmaarrfk commented Oct 3, 2024

View reviewed changes

recipe/install.sh Outdated Show resolved Hide resolved

hmaarrfk commented Oct 3, 2024

View reviewed changes

recipe/install.sh Outdated Show resolved Hide resolved

self review

ddb875f

hmaarrfk changed the title ~~Try to fix the shebang in certain scripts~~ Fix the shebang in certain scripts Oct 3, 2024

hmaarrfk mentioned this pull request Oct 3, 2024

[bot-automerge] libvips v8.15.4 conda-forge/libvips-feedstock#119

Closed

3 tasks

pkgw reviewed Oct 14, 2024

View reviewed changes

hmaarrfk added 3 commits October 14, 2024 11:11

Try without the replacement to test the test

7f28508

try to make the grep thing work.

ef61dca

update comment

ab6ee65

hmaarrfk commented Oct 14, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

hmaarrfk added 2 commits October 14, 2024 12:15

just use a dot.

84c8221

Update the build script

cc034a5

pkgw merged commit 7875eb8 into conda-forge:main Oct 14, 2024
8 checks passed

This was referenced Jan 7, 2025

Incorrect Python shebang in Windows package for glib-genmarshal, glib-mkenums, gtester-report #198

Closed

Manual harfbuzz 10 migration conda-forge/gtk3-feedstock#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the shebang in certain scripts #192

Fix the shebang in certain scripts #192

hmaarrfk commented Sep 28, 2024 •

edited

Loading

github-actions bot commented Sep 28, 2024 •

edited by conda-forge-admin

Loading

hmaarrfk Sep 29, 2024

hmaarrfk Sep 29, 2024

ehfd Oct 3, 2024 •

edited

Loading

ehfd commented Oct 3, 2024

hmaarrfk commented Oct 3, 2024

hmaarrfk commented Oct 3, 2024

isuruf Oct 3, 2024

hmaarrfk Oct 3, 2024

pkgw Oct 14, 2024

hmaarrfk Oct 14, 2024

hmaarrfk Oct 14, 2024

pkgw Oct 14, 2024 •

edited

Loading

hmaarrfk Oct 14, 2024

ehfd commented Oct 13, 2024

pkgw left a comment

pkgw Oct 14, 2024

pkgw Oct 14, 2024

hmaarrfk commented Oct 14, 2024

pkgw commented Oct 14, 2024

Fix the shebang in certain scripts #192

Fix the shebang in certain scripts #192

Conversation

hmaarrfk commented Sep 28, 2024 • edited Loading

github-actions bot commented Sep 28, 2024 • edited by conda-forge-admin Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehfd Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

ehfd commented Oct 3, 2024

hmaarrfk commented Oct 3, 2024

hmaarrfk commented Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkgw Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehfd commented Oct 13, 2024

pkgw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmaarrfk commented Oct 14, 2024

pkgw commented Oct 14, 2024

hmaarrfk commented Sep 28, 2024 •

edited

Loading

github-actions bot commented Sep 28, 2024 •

edited by conda-forge-admin

Loading

ehfd Oct 3, 2024 •

edited

Loading

pkgw Oct 14, 2024 •

edited

Loading