#25942 closed defect (fixed)

Fix win32 test failure for crypto/openssl_version

Reported by: isis Owned by:
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version: Tor: 0.3.3.5-rc
Severity: Normal Keywords: tor-ci tor-windows tor-testing
Cc: Actual Points:
Parent ID: #25549 Points: 1
Reviewer: Sponsor:

Description

From #25549, the crypto/openssl_version test fails on win32 builds with:

crypto/openssl_version: [forking] openssl version = 1.0.2l
openssl h_version = 1.0.2n
  FAIL ../src/test/test_crypto.c:156: assert(!strcmpstart(version, h_version))
  [openssl_version FAILED]

This could possibly be an artifact of how the CI environment is set up. If so, there should be a CI_WINDOWS environment variable set on the machine, so we can probably just ignore this test?

Child Tickets

Change History (13)

comment:1 Changed 12 months ago by nickm

I'm slightly confused: I thought this bug was the reason that you started building libevent from scratch. That didn't help?

comment:2 in reply to:  1 Changed 12 months ago by isis

Replying to nickm:

I'm slightly confused: I thought this bug was the reason that you started building libevent from scratch. That didn't help?


Nope, it strangely did not. The AppVeyor docs say that the images have OpenSSL 1.0.2L installed… but say nothing about headers. I don't know much about mingw/msys2… could it be possible that it's including its own openssl headers somewhere? (That sounds really strange, but I'd frankly not put it past it given how strange the rest of the platform/compiler is. Also, otherwise, I have no idea where the headers would be coming from since we're not installing anything other than libevent 2.1.8-stable.)

comment:3 Changed 12 months ago by nickm

So, does that mean we should we revert the build-our-own libevent change?

comment:4 in reply to:  3 Changed 12 months ago by isis

Replying to nickm:

So, does that mean we should we revert the build-our-own libevent change?


Eh… that apparently means going back to using pacman, which—due to Arch's Terrible Ideas About Operating Systems—means there's no way to specify a version (since Arch/pacman have "rolling releases"). So that would mean at some point in the future we'd end up with a different libevent on the CI machines, which I think generally moving parts in CI is bad? The caching should kick in once we have a successful build, and then IIUC we shouldn't have to build libevent ever again until we change the git tag listed in the .appveyor-libevent-version file.

comment:5 Changed 12 months ago by saper

I think that this is related to two openssl's being installed, one from MSYS, the other one from MinGW. One comes with headers, but the other one comes with DLL. I thought once I understand the theory between MSYS and MinGW ("One is just enough unix to let you run configure, the other one is real stuff you link you app with") but this is kind of confusing.

comment:6 Changed 12 months ago by saper

If there is anyone to blame are those cool Linux architects who invented "devel" packages.

(I think this is because there is no mingw-w64-x86_64-openssl-devel pendant to mingw-w64-x86_64-openssl, as there is openssl-devel for openssl).

comment:7 Changed 11 months ago by catalyst

Parent ID: #25549

comment:8 Changed 11 months ago by catalyst

Status: newmerge_ready

Thanks! This looks like it resolves the openssl_version test but the other test failures that are children of #25549 are still failing.

comment:9 Changed 11 months ago by nickm

Milestone: Tor: 0.3.4.x-final

comment:10 Changed 11 months ago by nickm

I think I'm supposed to wait on this one until the parent ticket is ready -- but please let me know if I'm wrong about that.

comment:11 Changed 11 months ago by isis

I think this can get merged? It looks like there's just one more error that saper describes here, but I don't know the full details.

comment:12 in reply to:  11 Changed 11 months ago by catalyst

Replying to isis:

I think this can get merged? It looks like there's just one more error that saper describes here, but I don't know the full details.

I think it depends on #25549 because we don't have a .appveyor.yml merged yet and that's the ticket with a pointer to the actual patch.

comment:13 Changed 11 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Fix merged as part of parent

Note: See TracTickets for help on using tickets.