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

got: update to 0.98.2 #23716

Closed
wants to merge 1 commit into from

Conversation

artkiver
Copy link
Contributor

Description

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 14.4.1 23E224 arm64
Command Line Tools 15.3.0.0.1.1708646388

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@artkiver
Copy link
Contributor Author

artkiver commented Apr 25, 2024

It appears as if GitHub Actions CI is defaulting to OpenSSL(3), but is failing with libtls not being found from the build logs?

I tried modifying the Portfile locally to be a bit more choosey cribbing from rpki-client as follows:

    depends_lib-append  path:lib/libtls.dylib:libressl
}

variant openssl conflicts libressl description {use OpenSSL} {
    depends_lib-append  port:libretls \
                        port:openssl
}

if {![catch {registry_active openssl}] && ![variant_isset libressl]} {
 # openssl is installed and the libressl variant isn't manually requested, default to openssl
  default_variants +openssl
} elseif {(![catch {registry_active libressl}] || ![catch {registry_active libressl-devel}]) && ![variant_isset openssl]} {
 # either libressl or libressl-devel are already installed, and the openssl variant is not explicitly requested, default to libressl
  default_variants +libressl
} elseif {![variant_isset openssl]} {
 # neither openssl nor libressl-devel are installed, and the openssl variant isn't selected, so libressl should be the default
  default_variants +libressl
}

In theory, that should install libretls as a dependency, only if OpenSSL is installed, thus providing the libtls that is otherwise missing and throwing an error in the GitHub Actions CI build logs.

However, in practice, that approach is now breaking builds for me locally and seems to be installing OpenSSL even when I specify the +libressl variant explicitly. Which, is a bit beyond me?

I'm not sure what else has changed in MacPorts nor GitHub Actions CI since the last time I updated Got, but I am guessing it may be related to the breakage also observed with LibreSSL 3.9.1 on older versions of OS X as mentioned in https://trac.macports.org/ticket/69490

I am inclined to agree with @ryandesign here: https://trac.macports.org/ticket/69490#comment:2 though coordination on that scale is a bit of an undertaking and the temptation to simply force Got to use LibreSSL via something as simple as

depends_lib port:libressl \

In the Portfile is strong, since that works for me locally without issues.

It does beg the question: did something change with MacPorts or GitHub Actions CI with how:

depends_lib path:lib/libtls.dylib:openssl \

Is handled in the last several months?

@artkiver
Copy link
Contributor Author

I've opened a Trac ticket and included some various different Portfile modifications here:

https://trac.macports.org/ticket/69827

Ideally something such as the https://trac.macports.org/raw-attachment/ticket/69827/Portfile.got0.98.2libreoropenssl method could be improved upon to function more reliably, but it begs the question: if the dylib approach was working previously, what changed in MacPorts or GitHub Actions CI or the build bot to have broken this since the last time Got was updated?

@ryandesign
Copy link
Contributor

I'm not sure what else has changed

What seems to have changed is that got-portable 0.98 now requires libretls:

ThomasAdam/got-portable@a58e44b#diff-49473dca262eeab3b4a43002adb08b4db31020d190caaad1594b47f1d5daa810

so add the port:libretls library dependency to the portfile.

There is a comment there that the libretls dependency may need to be made OS-specific. If you believe it may not actually be required on macOS you could try removing the check for it and seeing if you can still build. If so, you can provide that information to the developers of got-portable to consider for the next version.

@artkiver
Copy link
Contributor Author

I'm not sure what else has changed

What seems to have changed is that got-portable 0.98 now requires libretls:

ThomasAdam/got-portable@a58e44b#diff-49473dca262eeab3b4a43002adb08b4db31020d190caaad1594b47f1d5daa810

so add the port:libretls library dependency to the portfile.

There is a comment there that the libretls dependency may need to be made OS-specific. If you believe it may not actually be required on macOS you could try removing the check for it and seeing if you can still build. If so, you can provide that information to the developers of got-portable to consider for the next version.

I sincerely wish it were as simple as adding port:libretls to the Portfile, if it were, I would have done so already.

Though for completeness, I shared the results of such an attempt here:

https://trac.macports.org/ticket/69490#comment:5

@artkiver
Copy link
Contributor Author

I updated the Trac issue with some more experiments here:

https://trac.macports.org/ticket/69827#comment:1

At the moment the most recent Portfile attached there will pass %port -v install and %port test but errors out %port -vst install.

I've included other details in the Trac ticket as well as some things that are confusing me. I have refactored it various other ways, but I think that approach might at least pass the GitHub Actions' CI?

I still feel as if I am missing something obvious, or if not obvious there is probably a better way to phrase:

if {[catch {registry_active openssl}] && ![variant_isset libressl]} {
  # openssl is installed and the libressl variant isn't manually requested, default to openssl
  default_variants +openssl
} elseif {([catch {registry_active libressl}] || [catch {registry_active libressl-devel}]) && ![variant_isset openssl]} {
  # either libressl or libressl-devel are already installed, and the openssl variant is not explicitly requested, default to libressl
  default_variants +libressl
}

Yet I have tried various other permutations and they mostly seem even less functional. ;-/

@artkiver
Copy link
Contributor Author

artkiver commented May 6, 2024

This has been superseded by the following PR for got 0.99, and some other changes to the Portfile which at least make it pass the GitHub Actions CI tests; though it isn't quite satisfying for LibreSSL users, hopefully they'll read the comment and modify the Portfile manually until I can sort out something better as an improvement:

#23847

@artkiver artkiver closed this May 6, 2024
@artkiver artkiver deleted the got-portable0.98.2_uno branch May 7, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants