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

Unexpected behavior from patches if filename extention not detected #1418

Open
chrisburr opened this issue Feb 9, 2025 · 4 comments
Open

Comments

@chrisburr
Copy link

When the URL encodes the filename in a non-obvious way the sources are not extracted and instead you get the raw tarball. I'm not sure if this is intentional but if it is it's not intuitive to me. Adding file_name: YODA-${{ version }}.tar.gz makes it a bit more clean but it's still annoying to manually extract the tarball.

I guess the bug here is that the file_name key is not respected when detecting how to extract the sources? Also I think the file_name key should be mandatory if the URL contains parameters or if the filename doesn't contain .. Alternatively the detection could be done based on the content of the file.

Debugging this led me to thinking there should be a extract key under sources to enable/disable extracting the sources. Defaults to null which means detect it based on the source content. If extract: false then the patches key shouldn't be available.

schema_version: 1

context:
  name: yoda
  version: 2.0.1

package:
  name: ${{ name|lower }}
  version: ${{ version }}

source:
  url: https://yoda.hepforge.org/downloads?f=YODA-${{ version }}.tar.gz
  sha256: 28a900b62b0ac967edfd1a64196c1518a1adcec12b08da5ff4ae5adff6a69662
  patches:
    - 0001-Fix-setting-rpath-when-installing-on-macOS.patch
0001-Fix-setting-rpath-when-installing-on-macOS.patch

From 05ddce98340bbd47ff15b54e4bbe6d561880a50f Mon Sep 17 00:00:00 2001
From: Chris Burr <[email protected]>
Date: Sun, 9 Feb 2025 05:43:56 +0100
Subject: [PATCH] Fix setting rpath when installing on macOS

---
 pyext/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pyext/Makefile.am b/pyext/Makefile.am
index 5867aa05..98feac73 100644
--- a/pyext/Makefile.am
+++ b/pyext/Makefile.am
@@ -29,7 +29,7 @@ install-exec-local: build/yoda/core.so
 	install_name_tool -change \
         $(abs_top_srcdir)/src/.libs/libYODA.dylib \
         $(libdir)/libYODA.dylib \
-        $(abs_builddir)/build/yoda/core.so
+        $(DESTDIR)$(YODA_PYTHONPATH)/core.so

 else

--
2.47.0

@wolfv
Copy link
Member

wolfv commented Feb 12, 2025

Maybe you can use the link from Gitlab instead?

https://gitlab.com/hepcedar/yoda/-/archive/yoda-2.0.3/yoda-yoda-2.0.3.tar.gz

I am not sure we can reasonably deduce file names from a query string. It seems pretty brittle. However, the download comes with the correct Content-Disposition header.

Content-Disposition: attachment; filename=YODA-2.0.1.tar.gz

Which we could use instead of looking only at the URL.

@chrisburr
Copy link
Author

Maybe you can use the link from Gitlab instead?

Nice find! Though I know other packages with similar issues so the general case still applies.

I am not sure we can reasonably deduce file names from a query string.

Yeah I don't expect you'll be able to do that. I was expecting to need to set file_name: but perhaps the Content-Disposition: header works though I do wonder how that works from a reproducibility perspective.

I think the bug is more that not even this works:

source:
  url: https://yoda.hepforge.org/downloads?f=YODA-${{ version }}.tar.gz
  sha256: 28a900b62b0ac967edfd1a64196c1518a1adcec12b08da5ff4ae5adff6a69662
  file_name: YODA-${{ version }}.tar.gz
  patches:
    - 0001-Fix-setting-rpath-when-installing-on-macOS.patch

I've often wondered about having a sources repo for conda-forge that could just be based on the SHA256. But that would mean any filename based decompression techniques wouldn't work. I wonder if the libmagic-style approach would be my preferred one...

@wolfv
Copy link
Member

wolfv commented Feb 13, 2025

The thing is that we don't extract the file when the file name is set explicitly. Sometimes you want an archive as archive eg for testing

@chrisburr
Copy link
Author

How about adding a key to control the extraction that defaults to autodetection?

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

No branches or pull requests

2 participants