Skip to content

Conversation

@nirs
Copy link
Contributor

@nirs nirs commented Sep 12, 2025

Since #65 we don't need to change the install name.

Example build:

% make
cargo build --release
    ...
    Finished `release` profile [optimized] target(s) in 6.19s
codesign --entitlements krunkit.entitlements --force -s - target/release/krunkit
target/release/krunkit: replacing existing signature

% otool -L target/release/krunkit
target/release/krunkit:
        /opt/homebrew/opt/libkrun-efi/lib/libkrun-efi.1.dylib (compatibility version 1.0.0, current version 1.15.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3502.1.255)
        /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

Example build with LIBKRUN_EFI:

% make LIBKRUN_EFI=/usr/local/lib/libkrun-efi.dylib
cargo build --release
    ...
    Finished `release` profile [optimized] target(s) in 5.66s
codesign --entitlements krunkit.entitlements --force -s - target/release/krunkit
target/release/krunkit: replacing existing signature

% otool -L target/release/krunkit
target/release/krunkit:
        /usr/local/lib/libkrun-efi.1.dylib (compatibility version 1.0.0, current version 1.15.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3502.1.255)
        /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

Since containers#65 we don't need to change the install name.

Example build:

    % make
    cargo build --release
        ...
        Finished `release` profile [optimized] target(s) in 6.19s
    codesign --entitlements krunkit.entitlements --force -s - target/release/krunkit
    target/release/krunkit: replacing existing signature

    % otool -L target/release/krunkit
    target/release/krunkit:
            /opt/homebrew/opt/libkrun-efi/lib/libkrun-efi.1.dylib (compatibility version 1.0.0, current version 1.15.0)
            /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3502.1.255)
            /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
            /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
            /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
            /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

Example build with LIBKRUN_EFI:

    % make LIBKRUN_EFI=/usr/local/lib/libkrun-efi.dylib
    cargo build --release
        ...
        Finished `release` profile [optimized] target(s) in 5.66s
    codesign --entitlements krunkit.entitlements --force -s - target/release/krunkit
    target/release/krunkit: replacing existing signature

    % otool -L target/release/krunkit
    target/release/krunkit:
            /usr/local/lib/libkrun-efi.1.dylib (compatibility version 1.0.0, current version 1.15.0)
            /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3502.1.255)
            /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
            /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
            /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
            /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

Signed-off-by: Nir Soffer <nirsof@gmail.com>
@nirs
Copy link
Contributor Author

nirs commented Sep 12, 2025

/cc @slp
/cc @tylerfanelli
/cc @jakecorrenti

Copy link
Member

@jakecorrenti jakecorrenti left a comment

Choose a reason for hiding this comment

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

Thanks @nirs, sorry for the delay.

Could you also update the README.md as well?

@nirs
Copy link
Contributor Author

nirs commented Sep 22, 2025

Thanks @nirs, sorry for the delay.

Could you also update the README.md as well?

This internal change was not mentioned in README.md, and make LIBKRUN_EFI= is already there.

@jakecorrenti
Copy link
Member

Thanks @nirs, sorry for the delay.
Could you also update the README.md as well?

This internal change was not mentioned in README.md, and make LIBKRUN_EFI= is already there.

Yes, but your changes would mean that make LIBKRUN_EFI=<path> is a NOOP, no? The user would have to do something like LIBKRUN_EFI=<path> make to set the environment variable the build.rs file cares about.

@nirs
Copy link
Contributor Author

nirs commented Sep 22, 2025

Variables defined in the command line are passes to the environment of child processes created by make. Maybe we should document it in the Makefile, or define the default in the Makefile instead of build.rs.

See https://www.gnu.org/software/make/manual/html_node/Variables_002fRecursion.html

@jakecorrenti
Copy link
Member

Variables defined in the command line are passes to the environment of child processes created by make. Maybe we should document it in the Makefile, or define the default in the Makefile instead of build.rs.

See https://www.gnu.org/software/make/manual/html_node/Variables_002fRecursion.html

Ah, ok. Thank you for the clarification, I wasn't aware of how that worked.

I think it's fine as is and we can merge :)

@jakecorrenti jakecorrenti merged commit 193876a into containers:main Sep 24, 2025
3 checks passed
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.

2 participants