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

Add Windows binary packaging / installers. #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

slproweb
Copy link

I added two files to this commit that differ from the original repository: LICENSE.txt from the openssl/openssl repository and ms-windows/support/SOURCES.txt to select the MIT license for the support libraries (I assume MIT is fine).

Creating New Templates
----------------------

When a new base version is released (e.g. 3.2), the simplest approach to creating a new template is to copy the previous version's template and then make some adjustments to 'info.json' and various installer scripts. For official releases, the various dependencies should be updated to best reflect supported end-user OSes and architectures. The build tools script can be used to evaluate/validate all potential dependencies via 'init-test':
Copy link
Member

Choose a reason for hiding this comment

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

Note: I'm wondering if the "just copy the last template and adjust" could be templated...

I am not trying to say that it should be done in this PR! Consider this a collective mental note for future enhancements

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

This looks great. Mostly minor comments.

ms-windows/README.md Show resolved Hide resolved
ms-windows/README.md Show resolved Hide resolved
ms-windows/README.md Show resolved Hide resolved
The files in this directory come from these open source libraries:

https://github.com/cubiclesoft/php-misc (MIT license)
https://github.com/cubiclesoft/ultimate-web-scraper (MIT license)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, but I'm not sure we've done this before (i.e. had something with a different licence to all the rest of the files in the tree). We might need to double check with OMC that this is ok.

Since the files in this directory are just imports of tools from somewhere else, I've not reviewed their contents.

Copy link
Member

Choose a reason for hiding this comment

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

I'll get this raised with OMC so we can confirm this is ok.

ms-windows/build-tools.php Show resolved Hide resolved
ms-windows/templates/1.1.1/x86/openssl.iss Outdated Show resolved Hide resolved
ms-windows/templates/3.0/x64/openssl.iss Show resolved Hide resolved
ms-windows/templates/3.0/x86/openssl.iss Show resolved Hide resolved
ms-windows/templates/3.1/x64/openssl.iss Show resolved Hide resolved
Compression=bzip
ArchitecturesAllowed=x64
ArchitecturesInstallIn64BitMode=x64
; Minimum version supported is Windows XP.
Copy link
Member

Choose a reason for hiding this comment

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

I do wonder whether we can bump this up for 3.1 (maybe even 3.0). There are certain optimisations and benefits that we get if we compile OpenSSL targeting a more recent version of Windows. People that want XP support can still build from source.

Copy link
Author

Choose a reason for hiding this comment

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

If this is the route to take, Inno Setup 5 can be dropped altogether. When setting up the Inno Setup 5 dependency, it triggers the UAC prompt even though it ends up doing nothing requiring elevation. Dropping that dependency cleans up the initialization phase. Inno Setup 5 usage exists for XP support. So if we don't need that, the dependency can be dropped and the info.json files and the installers adjusted accordingly for Inno Setup 6.

Let me know what OSes to support for which versions.

@arapov arapov unassigned t8m Jun 28, 2023
@levicki
Copy link

levicki commented Jul 5, 2023

@mattcaswell

Is there a specific reason why InnoSetup script is not downloading / installing VC++ 2015-2022 redistributable instead of VC++ 2017?

AFAIK, 2015-2022 is fully compatible.

@@ -0,0 +1,1564 @@
<?php
/*
* Copyright 2016-2019 The OpenSSL Project Authors. All Rights Reserved.
Copy link
Member

Choose a reason for hiding this comment

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

2016-2019? Why 2016? At least 2019 should probably be 2023

DeleteDirectory($pathmap["[[SOURCE_TEMP_DIR]]"]);

// Clone the source tree to the temporary architecture directory.
// Doing this allows multiple architectures for a base version to be built simultaneously.
Copy link
Member

Choose a reason for hiding this comment

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

I won't claim it's never had a bug or won't ever again - but I don't recall seeing a bug about this recently. Anyway - I'm happy to defer this as a potential future enhancement.

if (!file_exists($pathmap["[[SOURCE_TEMP_DIR]]"] . "/makefile")) CLI::DisplayError($echoprefix . " Failed to generate 'makefile'.");


// Nothing is perfect. Patch the makefile.
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm. Somehow applying patches to our own build goes against the grain. But, ok, future enhancement.

echo $echoprefix . " Finished build at " . date("Y-m-d H:i:s") . ". Build time: " . $mins . " min " . $secs . " sec\n";
}

// Run first build. This build takes much longer due to also building the test suite.
Copy link
Member

Choose a reason for hiding this comment

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

So we install the test suite? That's unusual. I'm in two minds about it. I'd be interested to hear the thoughts of other reviewers about this.

if (!is_dir($pathmap["[[BUILD_OUTPUT_DIR]]"] . "/dll_MTd")) RunBuild($ver, $arch, $baseenv, 4, $pathmap, $archinfo, "--debug shared no-tests", "/MTd", "dll_MTd", array());

if (!is_dir($pathmap["[[BUILD_OUTPUT_DIR]]"] . "/static_MD")) RunBuild($ver, $arch, $baseenv, 5, $pathmap, $archinfo, "no-shared no-tests", "/MD", "static_MD", array("apps"));
if (!is_dir($pathmap["[[BUILD_OUTPUT_DIR]]"] . "/static_MT")) RunBuild($ver, $arch, $baseenv, 6, $pathmap, $archinfo, "no-shared no-tests", "/MT", "static_MT", array("apps"));
Copy link
Member

Choose a reason for hiding this comment

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

t might make more sense to go back and evaluate the generated makefile to see if it can produce everything required in just one build cycle by executing the linker step several additional times.

Future enhancement...

The files in this directory come from these open source libraries:

https://github.com/cubiclesoft/php-misc (MIT license)
https://github.com/cubiclesoft/ultimate-web-scraper (MIT license)
Copy link
Member

Choose a reason for hiding this comment

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

I'll get this raised with OMC so we can confirm this is ok.

@mattcaswell mattcaswell added the hold: need omc decision The OMC needs to make a decision label Jul 7, 2023
@mattcaswell
Copy link
Member

OMC Question: This PR contains a number of files under the "support" directory which are copies of external open source tool dependencies which are separately licensed. There is a note about the different licence conditions that apply to those files. Are we ok to do this?

OMC Question: This PR currently supports Windows XP. Building for a later version might bring with it various optimisations. What minimum version of Windows should we support?

$data = str_replace($key, $val, $data);
}

// The 'no-shared' build should only generate statically linked libraries and executables.
Copy link
Member

Choose a reason for hiding this comment

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

no-shared no-module should build only static libs and executables, including the legacy provider being included inside libcrypto.

}

// The 'no-shared' build should only generate statically linked libraries and executables.
if (strpos($configuretype, "no-shared") !== false)
Copy link
Member

Choose a reason for hiding this comment

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

This should not be needed if no-shared is combined with no-module.

@@ -0,0 +1,3372 @@
##
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should be really shipping the Mozilla CA cert bundle.

Copy link

Choose a reason for hiding this comment

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

@t8m Certainly not unless the bundle is created from scratch during build from certdata.txt, but given how many people build OpenSSL (including potentially automated builds) Mozilla wouldn't be happy with so many fetches of that file.

On the other hand, shipping a stale bundle is IMO worse than shipping none at all. Perhaps add some readme describing where and how to get it for those users need more than just their own certificates?

return $result;
}
}
?>
Copy link
Member

Choose a reason for hiding this comment

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

Missing EOL at EOF. Would it make sense for these files coming from outside project(s) to make them a submodule instead? Are they used verbatim or are there any subsequent modifications?

Comment on lines +1258 to +1266
$indir = $pathmap["[[SOURCE_TEMP_DIR]]"];
$outdir2 = $pathmap["[[BUILD_OUTPUT_DIR]]"] . "/" . $outdir;
@mkdir($outdir2, 0775, true);
if (in_array("all", $copymodes) || in_array("apps", $copymodes)) CopyBuildFiles($indir, $outdir2, "apps", true);
if (in_array("all", $copymodes) || in_array("engines", $copymodes)) CopyBuildFiles($indir, $outdir2, "engines", false);
if (in_array("all", $copymodes) || in_array("fuzz", $copymodes)) CopyBuildFiles($indir, $outdir2, "fuzz", false);
if (in_array("all", $copymodes) || in_array("providers", $copymodes)) CopyBuildFiles($indir, $outdir2, "providers", false);
if (in_array("all", $copymodes) || in_array("test", $copymodes)) CopyBuildFiles($indir, $outdir2, "test", true);
if (in_array("all", $copymodes) || in_array("tools", $copymodes)) CopyBuildFiles($indir, $outdir2, "tools", true);
Copy link
Member

Choose a reason for hiding this comment

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

So this does not use make install at all... Not sure if this is a problem or not.

Comment on lines +9 to +11
AppName=OpenSSL Light (64-bit)
AppVersion=[[DOTTED_VERSION]]
AppVerName=OpenSSL [[DOTTED_VERSION]] Light (64-bit)
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure OpenSSL Light is right name for the installation. Perhaps OpenSSL Runtime vs. OpenSSL Full would be better?

Copy link

Choose a reason for hiding this comment

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

@t8m That one bothers me too.

However, Runtime kind of implies that it contains only .dll files without the openssl.exe binary.

How about OpenSSL User Setup (binaries only) and OpenSSL Developer Setup (full version with docs/headers/libs)?

That IMO conveys the intent more clearly.

AppName=OpenSSL Light (64-bit)
AppVersion=[[DOTTED_VERSION]]
AppVerName=OpenSSL [[DOTTED_VERSION]] Light (64-bit)
AppPublisher=OpenSSL Foundation
Copy link
Member

Choose a reason for hiding this comment

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

OpenSSL Software Foundation

@t8m
Copy link
Member

t8m commented Jul 12, 2023

OMC: We leave the minimum Windows version to XP for the 32 bit binary build. We will require Windows 7 for the 64 bit binary build.

Answer to the other question is deferred to the next meeting.

@t8m
Copy link
Member

t8m commented Jul 12, 2023

@slproweb Do you have download logs for the existing binaries to find out how many downloads come from Windows XP and Windows Vista? If so what percentage of these old Windows do you see?

@ghost
Copy link

ghost commented Jul 28, 2023

please make sure just a normal Zip option is available as well. I dont like running random installers, I would prefer just to extract and use the executables, similar to a Linux system. until then I am using this:

https://indy.fulgan.com/SSL

@t8m
Copy link
Member

t8m commented Jul 31, 2023

please make sure just a normal Zip option is available as well. I dont like running random installers, I would prefer just to extract and use the executables, similar to a Linux system. until then I am using this:

https://indy.fulgan.com/SSL

I assume we could have a build that would create statically linked openssl.exe and have that zipped.

@arapov arapov assigned arapov and unassigned mattcaswell Jan 3, 2024
@nhorman
Copy link
Contributor

nhorman commented Jun 7, 2024

Marking as inactive, will be closed at the end of 3.4 dev cycle barring further input

@slproweb
Copy link
Author

slproweb commented Jun 7, 2024

I apologize for my lack of attention to this project. I got seriously sick twice in the last year (first with some nasty upper respiratory infection that knocked me out for over a month and the second time with COVID) and in between bouts of illness was otherwise extremely busy with a new (unrelated) project. It looks like someone else has picked this up and is running with it, which is a good thing. My original intent was to drop a working set of scripts and then the project could just run with it from there with minimal attention on my behalf. Looks like you guys are going the NSIS route instead of the InnoSetup route. Either installer technology is fine. I was merely importing what I had already built up over many years.

Some notes/commentary though:

The comment by @ghost from earlier about ZIP files is probably still relevant. The scripts I wrote generated a unified ZIP file output as the last step. I don't see anything similar for the new installer.

MSI is really important for SCCM-like tools. You'll get a bunch of not-so-polite requests for MSI support until you implement it. The WiX toolset XML wrapper is convenient for wrapping a generated EXE installer inside of a MSI. There are weird quirks with MSI. MSIX is supposedly "the future" but I've not seen it in the wild (yet). So for the future, you might consider taking the EXE generated by NSIS and using a basic WiX toolset XML wrapper like the one here to generate a MSI wrapped EXE. It's not a perfect solution but it'll generally get you 99% of the way there. The InnoSetup script specifically looks for a special MSI option to avoid adding spurious entries to the Windows registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold: need omc decision The OMC needs to make a decision inactive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants