Add license file to .pkg installers #1085

lrandersson · 2025-10-09T18:32:39Z

Description

To resolve #1074

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

constructor/shar.py

lrandersson · 2025-10-09T18:39:17Z

What is the recommendation here for testing? Two ideas (I think option 1 is the most simple):

Update test_example_extra_files, add also a license file and verify that it exists in the test_install.sh script.
Add a new test similar to test_example_extra_files, but only for the purpose of checking that the license file exists.

Let me know if you have any other idea/preference.

marcoesters · 2025-10-09T18:59:25Z

What is the recommendation here for testing? Two ideas (I think option 1 is the most simple):
1. Update `test_example_extra_files`, add also a license file and verify that it exists in the `test_install.sh` script.

2. Add a new test similar to `test_example_extra_files`, but only for the purpose of checking that the license file exists.
Let me know if you have any other idea/preference.

I think adding an extra test just for the license file itself is overkill, so option 1 seems simple. However, I also notices that we have no test coverage for all these other files like the content of conda-meta/history and all those required files. This may be out of scope, but something to think about.

examples/extra_files/test_install.sh

lrandersson · 2025-10-10T18:33:34Z

Updated the test in 1dc095c so the error message when running the test actually points to which file is missing, below is the output from running the test locally (except that I forced it to fail for demonstration):

+ echo 'Added by test-install script'
+ missing=false
+ test -f '/tmp/pytest-of-root/pytest-3/test_example_extra_files0/i n s t a l l-ExtraFiles-X-Linux-aarch64-sh/more_data/README.md'
+ test -f '/tmp/pytest-of-root/pytest-3/test_example_extra_files0/i n s t a l l-ExtraFiles-X-Linux-aarch64-sh/something2.txt'
+ '[' SH = SH ']'
+ test -f '/tmp/pytest-of-root/pytest-3/test_example_extra_files0/i n s t a l l-ExtraFiles-X-Linux-aarch64-sh/LICENSE.txt'
+ echo 'Missing: /tmp/pytest-of-root/pytest-3/test_example_extra_files0/i n s t a l l-ExtraFiles-X-Linux-aarch64-sh/LICENSE.txt'
+ missing=true
+ '[' true = true ']'
+ exit 1
ERROR: executing post_install.sh failed

marcoesters · 2025-10-20T21:28:35Z

constructor/preconda.py

        list[os.PathLike]: List of normalized paths of copied locations.
    """
+
+    def check_file_exists(file_path):


Suggested change

def check_file_exists(file_path):

def check_file_exists(file_path: str) -> Path:

Let's try and add typing where we can. I'm not sure about that function name either - checking isn't really its primary purpose (or that would return a bool).

Thanks for catching that, will make sure I do that in the future.
I've applied all the changes from the first review, see 436cae7.
For some reason it doesn't work to "Add suggestion to batch", even when I go to "File mode" as the UI suggests.

You may have to switch between the old and new UI to accept suggestions.

marcoesters · 2025-10-20T21:32:13Z

examples/extra_files/test_install.sh

+test -f "$PREFIX/more_data/README.md" || { echo "Missing: $PREFIX/more_data/README.md"; missing=true; }
+test -f "$PREFIX/something2.txt" || { echo "Missing: $PREFIX/something2.txt"; missing=true; }


Using proper if statements would make the code easier to read.

Fully agree, I changed all of them.

marcoesters · 2025-10-20T21:32:41Z

examples/extra_files/test_install.sh

+
+# Ideally we should test the .pkg and .sh installers separately since
+# the current behavior for .sh-installers is to include but also rename the license file to LICENSE.txt,
+# but for macOS the name of the provided license file remains unchanged.


Suggested change

# but for macOS the name of the provided license file remains unchanged.

# but for .pkg the name of the provided license file remains unchanged.

marcoesters · 2025-10-20T21:34:07Z

news/1085-include-installer-license-file

@@ -0,0 +1,19 @@
+### Enhancements
+
+* The license file is now also included in the installation directory for macOS package (.pkg) installers.


Suggested change

* The license file is now also included in the installation directory for macOS package (.pkg) installers.

* Include the license file in PKG installers. (#1074 via #1085)

marcoesters · 2025-10-20T21:34:32Z

constructor/preconda.py

-            orig_path = Path(origin)
-            if not orig_path.exists():
-                raise FileNotFoundError(f"File {origin} does not exist.")
+


Suggested change

Let's keep diffs small by not adding additional blank lines where they're not necessary.

marcoesters · 2025-10-20T21:34:45Z

constructor/shar.py


    tarball = join(tmp_dir, "pkgs", "tmp.tar")
    t = tarfile.open(tarball, "w")
+


Suggested change

marcoesters · 2025-10-20T21:34:50Z

constructor/shar.py

+
    t.add(preconda_tarball, basename(preconda_tarball))
    t.add(postconda_tarball, basename(postconda_tarball))
+


Suggested change

marcoesters · 2025-10-20T21:35:31Z

constructor/shar.py

+
    if "license_file" in info:
        t.add(info["license_file"], "LICENSE.txt")
+


Suggested change

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Oct 9, 2025

lrandersson commented Oct 9, 2025

View reviewed changes

constructor/shar.py Outdated Show resolved Hide resolved

conda-bot added this to 🔎 Review Oct 9, 2025

github-project-automation bot moved this to 🆕 New in 🔎 Review Oct 9, 2025

lrandersson commented Oct 9, 2025

View reviewed changes

examples/extra_files/test_install.sh Outdated Show resolved Hide resolved

lrandersson changed the title ~~[WIP] Add license file to .pkg installers~~ Add license file to .pkg installers Oct 9, 2025

lrandersson marked this pull request as ready for review October 9, 2025 20:04

lrandersson requested a review from a team as a code owner October 9, 2025 20:04

lrandersson force-pushed the dev-ra-include-license-file branch from 1dc095c to c82e1c8 Compare October 14, 2025 12:29

lrandersson added 5 commits October 14, 2025 14:45

Add license file to .pkg installers

8458252

Add test

c32f4b9

Add news

a767526

Rename file, test difference between .sh/.pkg

e5b6965

Updated test to point at which file that is missing

1da6431

lrandersson force-pushed the dev-ra-include-license-file branch from c82e1c8 to 1da6431 Compare October 14, 2025 18:46

lrandersson added 3 commits October 14, 2025 15:09

improve error for file copy operation, fix issue in windows test

94fb2b9

Update from pre-commit

e1860f3

Fix issue with string being passed instead of a list

941c278

marcoesters reviewed Oct 20, 2025

View reviewed changes

add fixes from review

436cae7

lrandersson force-pushed the dev-ra-include-license-file branch from 626b5e6 to 436cae7 Compare October 21, 2025 12:30

lrandersson requested a review from marcoesters October 21, 2025 12:32

	def check_file_exists(file_path):
	def check_file_exists(file_path: str) -> Path:

		test -f "$PREFIX/more_data/README.md" \|\| { echo "Missing: $PREFIX/more_data/README.md"; missing=true; }
		test -f "$PREFIX/something2.txt" \|\| { echo "Missing: $PREFIX/something2.txt"; missing=true; }

	# but for macOS the name of the provided license file remains unchanged.
	# but for .pkg the name of the provided license file remains unchanged.

		@@ -0,0 +1,19 @@
		### Enhancements

		* The license file is now also included in the installation directory for macOS package (.pkg) installers.

	* The license file is now also included in the installation directory for macOS package (.pkg) installers.
	* Include the license file in PKG installers. (#1074 via #1085)


		tarball = join(tmp_dir, "pkgs", "tmp.tar")
		t = tarfile.open(tarball, "w")


		t.add(preconda_tarball, basename(preconda_tarball))
		t.add(postconda_tarball, basename(postconda_tarball))


		if "license_file" in info:
		t.add(info["license_file"], "LICENSE.txt")

Add license file to .pkg installers #1085

Are you sure you want to change the base?

Add license file to .pkg installers #1085

Conversation

lrandersson commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist - did you ...

Uh oh!

Uh oh!

lrandersson commented Oct 9, 2025

Uh oh!

marcoesters commented Oct 9, 2025

Uh oh!

Uh oh!

lrandersson commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lrandersson commented Oct 9, 2025 •

edited

Loading