Skip to content

Commit

Permalink
[release/5.0-preview4] Revert processing bundles in framework (#35679)
Browse files Browse the repository at this point in the history
This commit reverts:
Revert "Single-File: Process bundles in the framework (#34274)"
This reverts commit 78b303d.

Revert "Single-File Bundler: Add a FileSize test (#35149)"
This reverts commit 779588a.

*Customer Scenario*

Publishing apps as a self-contained single-file doesn't work as expected.

* Publish needs to generate hostpolicy and hostfxr separate from the single file bundle
* Cross-platform publishing is incorrect

*Problem*

Since Static-apphost is not yet ready, processing bundle content in hostpolicy means that  hostpolicy and hostfxr DLLs need to be separate from the bundle. This causes self-contained single-file apps to not be a "single file" temporarily. 

The change also requires supporting changes from the SDK, to publish hostfxr and hostpolicy as separate files, and to invoke HostModel library with arguments that facilitate cross-platform publishing.

*Solution*

To solve these, problem, this change reverts:

Revert "Single-File: Process bundles in the framework (#34274)" commit 78b303d.

and a dependent test-only change:

Revert "Single-File Bundler: Add a FileSize test (#35149)" commit 779588a.

*Risk*

Medium
The change is contained to only host components: apphost, hostpolicy, and hostfxr.
However, the change is big, and needs testing in runtime and SDK repos.

*Testing*

Manually tested the SDK by inserting apphost, hostfxr, hostpolicy, and hostmodel library from this build into the `dotnet/packs` preview-4 SDK from dotnet/sdk#11518 build.

Verified that:
* Singlefile apps can be published and run OK for  { Windows, Linux, Osx } x {netcoreapp3.0, netcoreapp3.1, netcoreapp5.0} 
* Cross-targeting builds of single-file apps build and run OK (ex: built on Windos, run on Mac).
  • Loading branch information
swaroop-sridhar committed May 1, 2020
1 parent 6d1f7e0 commit 47ec733
Show file tree
Hide file tree
Showing 67 changed files with 477 additions and 1,113 deletions.
19 changes: 17 additions & 2 deletions src/installer/corehost/cli/apphost/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,26 @@ endif()
set(SKIP_VERSIONING 1)

set(SOURCES
./bundle_marker.cpp
./bundle/file_entry.cpp
./bundle/manifest.cpp
./bundle/header.cpp
./bundle/marker.cpp
./bundle/reader.cpp
./bundle/extractor.cpp
./bundle/runner.cpp
./bundle/dir_utils.cpp
)

set(HEADERS
./bundle_marker.h
./bundle/file_type.h
./bundle/file_entry.h
./bundle/manifest.h
./bundle/header.h
./bundle/marker.h
./bundle/reader.h
./bundle/extractor.h
./bundle/runner.h
./bundle/dir_utils.h
)

if(CLR_CMAKE_TARGET_WIN32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,7 @@ void extractor_t::extract_new(reader_t& reader)
begin();
for (const file_entry_t& entry : m_manifest.files)
{
if (entry.needs_extraction())
{
extract(entry, reader);
}
extract(entry, reader);
}
commit_dir();
}
Expand All @@ -214,11 +211,6 @@ void extractor_t::verify_recover_extraction(reader_t& reader)

for (const file_entry_t& entry : m_manifest.files)
{
if (!entry.needs_extraction())
{
continue;
}

pal::string_t file_path = ext_dir;
append_path(&file_path, entry.relative_path().c_str());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,3 @@ file_entry_t file_entry_t::read(reader_t &reader)

return entry;
}

bool file_entry_t::needs_extraction() const
{
switch (m_type)
{
// Once the runtime can load assemblies from bundle,
// file_type_t::assembly should be in this category
case file_type_t::deps_json:
case file_type_t::runtime_config_json:
return false;

default:
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ namespace bundle
int64_t offset() const { return m_offset; }
int64_t size() const { return m_size; }
file_type_t type() const { return m_type; }
bool needs_extraction() const;

static file_entry_t read(reader_t &reader);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,29 @@

using namespace bundle;

bool header_fixed_t::is_valid() const
// The AppHost expects the bundle_header to be an exact_match for which it was built.
// The framework accepts backwards compatible header versions.
bool header_fixed_t::is_valid(bool exact_match) const
{
if (num_embedded_files <= 0)
{
return false;
}

// .net 5 host expects the version information to be 2.0
// .net core 3 single-file bundles are handled within the netcoreapp3.x apphost, and are not processed here in the framework.
return (major_version == header_t::major_version) && (minor_version == header_t::minor_version);
if (exact_match)
{
return (major_version == header_t::major_version) && (minor_version == header_t::minor_version);
}

return ((major_version < header_t::major_version) ||
(major_version == header_t::major_version && minor_version <= header_t::minor_version));
}

header_t header_t::read(reader_t& reader)
header_t header_t::read(reader_t& reader, bool need_exact_version)
{
const header_fixed_t* fixed_header = reinterpret_cast<const header_fixed_t*>(reader.read_direct(sizeof(header_fixed_t)));

if (!fixed_header->is_valid())
if (!fixed_header->is_valid(need_exact_version))
{
trace::error(_X("Failure processing application bundle."));
trace::error(_X("Bundle header version compatibility check failed."));
Expand All @@ -38,8 +44,10 @@ header_t header_t::read(reader_t& reader)
// bundle_id is a component of the extraction path
reader.read_path_string(header.m_bundle_id);

const header_fixed_v2_t *v2_header = reinterpret_cast<const header_fixed_v2_t*>(reader.read_direct(sizeof(header_fixed_v2_t)));
header.m_v2_header = *v2_header;
if (fixed_header->major_version > 1)
{
header.m_v2_header = reinterpret_cast<const header_fixed_v2_t*>(reader.read_direct(sizeof(header_fixed_v2_t)));
}

return header;
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ namespace bundle
uint32_t minor_version;
int32_t num_embedded_files;

bool is_valid() const;
bool is_valid(bool exact_match = false) const;
};
#pragma pack(pop)

// netcoreapp3_compat_mode flag is set on a .net5 app, which chooses to build single-file apps in .netcore3.x compat mode,
// This indicates that:
Expand All @@ -45,13 +46,12 @@ namespace bundle
netcoreapp3_compat_mode = 1
};

#pragma pack(push, 1)
struct location_t
{
public:
int64_t offset;
int64_t size;

bool is_valid() const { return offset != 0; }
};

// header_fixed_v2_t is available in single-file apps targetting .net5+ frameworks.
Expand All @@ -65,8 +65,6 @@ namespace bundle
location_t deps_json_location;
location_t runtimeconfig_json_location;
header_flags_t flags;

bool is_netcoreapp3_compat_mode() const { return (flags & header_flags_t::netcoreapp3_compat_mode) != 0; }
};
#pragma pack(pop)

Expand All @@ -76,25 +74,23 @@ namespace bundle
header_t(int32_t num_embedded_files = 0)
: m_num_embedded_files(num_embedded_files)
, m_bundle_id()
, m_v2_header()
, m_v2_header(NULL)

{
}

static header_t read(reader_t& reader);
const pal::string_t& bundle_id() const { return m_bundle_id; }
int32_t num_embedded_files() const { return m_num_embedded_files; }

const location_t& deps_json_location() const { return m_v2_header.deps_json_location; }
const location_t& runtimeconfig_json_location() const { return m_v2_header.runtimeconfig_json_location; }
bool is_netcoreapp3_compat_mode() const { return m_v2_header.is_netcoreapp3_compat_mode(); }
static header_t read(reader_t& reader, bool need_exact_version);
const pal::string_t& bundle_id() { return m_bundle_id; }
int32_t num_embedded_files() { return m_num_embedded_files; }

static const uint32_t major_version = 2;
static const uint32_t minor_version = 0;

private:
int32_t m_num_embedded_files;
pal::string_t m_bundle_id;
header_fixed_v2_t m_v2_header;
const header_fixed_v2_t* m_v2_header;

};
}
#endif // __HEADER_H__
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ manifest_t manifest_t::read(reader_t& reader, int32_t num_files)

for (int32_t i = 0; i < num_files; i++)
{
file_entry_t entry = file_entry_t::read(reader);
manifest.files.push_back(std::move(entry));
manifest.m_need_extraction |= entry.needs_extraction();
manifest.files.emplace_back(file_entry_t::read(reader));
}

return manifest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,9 @@ namespace bundle
class manifest_t
{
public:
manifest_t()
: m_need_extraction(false) {}

std::vector<file_entry_t> files;

static manifest_t read(reader_t &reader, int32_t num_files);

bool files_need_extraction() { return m_need_extraction; }

private:
bool m_need_extraction;
};
}
#endif // __MANIFEST_H__
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#include "bundle_marker.h"
#include "marker.h"
#include "pal.h"
#include "trace.h"
#include "utils.h"

int64_t bundle_marker_t::header_offset()
using namespace bundle;

int64_t marker_t::header_offset()
{
// Contains the bundle_placeholder default value at compile time.
// If this is a single-file bundle, the last 8 bytes are replaced
Expand All @@ -25,7 +27,7 @@ int64_t bundle_marker_t::header_offset()
0xee, 0x3b, 0x2d, 0xce, 0x24, 0xb3, 0x6a, 0xae
};

volatile bundle_marker_t* marker = reinterpret_cast<volatile bundle_marker_t *>(placeholder);
volatile marker_t* marker = reinterpret_cast<volatile marker_t *>(placeholder);

return marker->locator.bundle_header_offset;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#ifndef __BUNDLE_MARKER_H__
#define __BUNDLE_MARKER_H__
#ifndef __MARKER_H__
#define __MARKER_H__

#include <cstdint>

namespace bundle
{
#pragma pack(push, 1)
union bundle_marker_t
union marker_t
{
public:
uint8_t placeholder[40];
Expand All @@ -26,5 +28,5 @@
};
#pragma pack(pop)


#endif // __BUNDLE_MARKER_H__
}
#endif // __MARKER_H__
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

using namespace bundle;

const char* reader_t::add_without_overflow(const char* ptr, int64_t len)
const int8_t* reader_t::add_without_overflow(const int8_t* ptr, int64_t len)
{
const char* new_ptr = ptr + len;
const int8_t* new_ptr = ptr + len;

// The following check will fail in case len < 0 (which is also an error while reading)
// even if the actual arthmetic didn't overflow.
Expand Down Expand Up @@ -38,7 +38,7 @@ void reader_t::set_offset(int64_t offset)

void reader_t::bounds_check(int64_t len)
{
const char* post_read_ptr = add_without_overflow(m_ptr, len);
const int8_t* post_read_ptr = add_without_overflow(m_ptr, len);

// It is legal for post_read_ptr == m_bound_ptr after reading the last byte.
if (m_ptr < m_base_ptr || post_read_ptr > m_bound_ptr)
Expand Down Expand Up @@ -88,14 +88,11 @@ size_t reader_t::read_path_length()
return length;
}

size_t reader_t::read_path_string(pal::string_t &str)
void reader_t::read_path_string(pal::string_t &str)
{
const char* start_ptr = m_ptr;
size_t size = read_path_length();
std::unique_ptr<uint8_t[]> buffer{ new uint8_t[size + 1] };
read(buffer.get(), size);
buffer[size] = 0; // null-terminator
pal::clr_palstring(reinterpret_cast<const char*>(buffer.get()), &str);

return m_ptr - start_ptr; // This subtraction can't overflow because addition above is bounds_checked
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,19 @@ namespace bundle
// Helper class for reading sequentially from the memory-mapped bundle file.
struct reader_t
{
reader_t(const char* base_ptr, int64_t bound, int64_t start_offset = 0)
reader_t(const int8_t* base_ptr, int64_t bound)
: m_base_ptr(base_ptr)
, m_ptr(base_ptr)
, m_bound(bound)
, m_bound_ptr(add_without_overflow(base_ptr, bound))
{
set_offset(start_offset);
}

public:

void set_offset(int64_t offset);

operator const char*() const
operator const int8_t*() const
{
return m_ptr;
}
Expand All @@ -47,26 +46,26 @@ namespace bundle

// Return a pointer to the requested bytes within the memory-mapped file.
// Skip over len bytes.
const char* read_direct(int64_t len)
const int8_t* read_direct(int64_t len)
{
bounds_check(len);
const char *ptr = m_ptr;
const int8_t *ptr = m_ptr;
m_ptr += len;
return ptr;
}

size_t read_path_length();
size_t read_path_string(pal::string_t &str);
void read_path_string(pal::string_t &str);

private:

void bounds_check(int64_t len = 1);
static const char* add_without_overflow(const char* ptr, int64_t len);
static const int8_t* add_without_overflow(const int8_t* ptr, int64_t len);

const char* const m_base_ptr;
const char* m_ptr;
const int8_t* const m_base_ptr;
const int8_t* m_ptr;
const int64_t m_bound;
const char* const m_bound_ptr;
const int8_t* const m_bound_ptr;
};
}

Expand Down
Loading

0 comments on commit 47ec733

Please sign in to comment.