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.

Sign up for GitHub

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

Jump to bottom

build: Fix various shared library build issues. #41850

Closed
wants to merge 7 commits into from
Closed

build: Fix various shared library build issues. #41850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e76ddb4
build: fix various shared library build issues
lux01 Feb 4, 2022
8d55ff0
Fix bad merge
lux01 Mar 24, 2022
944e55a
Fixup linter complaint
mhdawson Mar 29, 2022
a840b01
Update tools/install.py
mhdawson Mar 29, 2022
72f166b
Generate and use a .def file. Add NODE_EXTERN_PRIVATE.
lux01 Apr 20, 2022
b3af563
Fix format-cpp and lint-cpp errors
lux01 Apr 26, 2022
aa9ea4d
Only export symbols with NODE_EXTERN_PRIVATE when building the shared…
lux01 Apr 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion node.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,16 @@
'dependencies': [ 'node_aix_shared' ],
}, {
'dependencies': [ '<(node_lib_target_name)' ],
'conditions': [
['OS=="win" and node_shared=="true"', {
'dependencies': ['generate_node_def'],
'msvs_settings': {
'VCLinkerTool': {
'ModuleDefinitionFile': '<(PRODUCT_DIR)/<(node_core_target_name).def',
},
},
}],
],
}],
[ 'node_intermediate_lib_type=="static_library" and node_shared=="false"', {
'xcode_settings': {
Expand Down Expand Up @@ -235,8 +245,15 @@
}],
[ 'node_shared=="true"', {
'xcode_settings': {
'OTHER_LDFLAGS': [ '-Wl,-rpath,@loader_path', ],
'OTHER_LDFLAGS': [ '-Wl,-rpath,@loader_path', '-Wl,-rpath,@loader_path/../lib'],
},
'conditions': [
['OS=="linux"', {
'ldflags': [
'-Wl,-rpath,\\$$ORIGIN/../lib'
],
}],
],
}],
[ 'enable_lto=="true"', {
'xcode_settings': {
Expand Down Expand Up @@ -749,6 +766,7 @@
'libraries': [
'Dbghelp',
'Psapi',
'Winmm',
'Ws2_32',
],
}],
Expand Down Expand Up @@ -1502,5 +1520,40 @@
},
]
}], # end aix section
['OS=="win" and node_shared=="true"', {
'targets': [
{
'target_name': 'gen_node_def',
'type': 'executable',
'sources': [
'tools/gen_node_def.cc'
],
},
{
'target_name': 'generate_node_def',
'dependencies': [
'gen_node_def',
'<(node_lib_target_name)',
],
'type': 'none',
'actions': [
{
'action_name': 'generate_node_def_action',
'inputs': [
'<(PRODUCT_DIR)/<(node_lib_target_name).dll'
],
'outputs': [
'<(PRODUCT_DIR)/<(node_core_target_name).def',
],
'action': [
'<(PRODUCT_DIR)/gen_node_def.exe',
'<@(_inputs)',
'<@(_outputs)',
],
},
],
},
],
}], # end win section
], # end conditions block
}
15 changes: 14 additions & 1 deletion node.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
[ 'clang==1', {
'cflags': [ '-Werror=undefined-inline', ]
}],
[ 'node_shared=="false" and "<(_type)"=="executable"', {
[ '"<(_type)"=="executable"', {
'msvs_settings': {
'VCManifestTool': {
'EmbedManifest': 'true',
Expand All @@ -41,6 +41,19 @@
'defines': [
'NODE_SHARED_MODE',
],
'conditions': [
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary for all platforms? I'm wondering since the only issues you mentioned for linux/osx don't seem related to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you may be right that these are only necessary on Windows. Their purpose is to ensure that libuv and v8 classes are correctly annotated with __declspec(dllimport) rather than __declspec(dllexport). I will refactor this condition block to only apply to Windows.

Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't hurt (and is more correct) to enable these on non-Windows.

['"<(_type)"=="executable"', {
'defines': [
'USING_UV_SHARED',
'USING_V8_SHARED',
'BUILDING_NODE_EXTENSION'
],
'defines!': [
'BUILDING_V8_SHARED=1',
'BUILDING_UV_SHARED=1'
]
}],
],
}],
[ 'OS=="win"', {
'defines!': [
Expand Down
6 changes: 3 additions & 3 deletions src/debug_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ template <typename... Args>
inline std::string SPrintF(const char* format, Args&&... args);
template <typename... Args>
inline void FPrintF(FILE* file, const char* format, Args&&... args);
void FWrite(FILE* file, const std::string& str);
void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);

// Listing the AsyncWrap provider types first enables us to cast directly
// from a provider type to a debug category.
Expand All @@ -57,7 +57,7 @@ enum class DebugCategory {
CATEGORY_COUNT
};

class EnabledDebugList {
class NODE_EXTERN_PRIVATE EnabledDebugList {
public:
bool enabled(DebugCategory category) const {
DCHECK_GE(static_cast<int>(category), 0);
Expand Down Expand Up @@ -168,7 +168,7 @@ void CheckedUvLoopClose(uv_loop_t* loop);
void PrintLibuvHandleInformation(uv_loop_t* loop, FILE* stream);

namespace per_process {
extern EnabledDebugList enabled_debug_list;
extern NODE_EXTERN_PRIVATE EnabledDebugList enabled_debug_list;

template <typename... Args>
inline void FORCE_INLINE Debug(DebugCategory cat,
Expand Down
2 changes: 1 addition & 1 deletion src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ class Environment;
struct AllocatedBuffer;

typedef size_t SnapshotIndex;
class IsolateData : public MemoryRetainer {
class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
public:
IsolateData(v8::Isolate* isolate,
uv_loop_t* event_loop,
Expand Down
10 changes: 10 additions & 0 deletions src/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@
# define NODE_EXTERN __attribute__((visibility("default")))
#endif

// Declarations annotated with NODE_EXTERN_PRIVATE do not form part of
// the public API. They are implementation details that can and will
// change between releases, even in semver patch releases. Do not use
// any such symbol in external code.
#ifdef NODE_SHARED_MODE
#define NODE_EXTERN_PRIVATE NODE_EXTERN
#else
#define NODE_EXTERN_PRIVATE
#endif

#ifdef BUILDING_NODE_EXTENSION
# undef BUILDING_V8_SHARED
# undef BUILDING_UV_SHARED
Expand Down
15 changes: 8 additions & 7 deletions src/node_internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,14 @@ enum InitializationSettingsFlags : uint64_t {
};

// TODO(codebytere): eventually document and expose to embedders.
InitializationResult InitializeOncePerProcess(int argc, char** argv);
InitializationResult InitializeOncePerProcess(
int argc,
char** argv,
InitializationSettingsFlags flags,
ProcessFlags::Flags process_flags = ProcessFlags::kNoFlags);
void TearDownOncePerProcess();
InitializationResult NODE_EXTERN_PRIVATE InitializeOncePerProcess(int argc,
char** argv);
InitializationResult NODE_EXTERN_PRIVATE InitializeOncePerProcess(
int argc,
char** argv,
InitializationSettingsFlags flags,
ProcessFlags::Flags process_flags = ProcessFlags::kNoFlags);
void NODE_EXTERN_PRIVATE TearDownOncePerProcess();
void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s);
void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s);
void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
Expand Down
2 changes: 1 addition & 1 deletion src/node_native_module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ using NativeModuleCacheMap =
// This class should not depend on any Environment, or depend on access to
// the its own singleton - that should be encapsulated in NativeModuleEnv
// instead.
class NativeModuleLoader {
class NODE_EXTERN_PRIVATE NativeModuleLoader {
public:
NativeModuleLoader(const NativeModuleLoader&) = delete;
NativeModuleLoader& operator=(const NativeModuleLoader&) = delete;
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ void Parse(
namespace per_process {

extern Mutex cli_options_mutex;
extern std::shared_ptr<PerProcessOptions> cli_options;
extern NODE_EXTERN_PRIVATE std::shared_ptr<PerProcessOptions> cli_options;

} // namespace per_process

Expand Down
2 changes: 1 addition & 1 deletion src/node_snapshot_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace node {
class ExternalReferenceRegistry;
struct SnapshotData;

class SnapshotBuilder {
class NODE_EXTERN_PRIVATE SnapshotBuilder {
public:
static std::string Generate(const std::vector<std::string> args,
const std::vector<std::string> exec_args);
Expand Down
6 changes: 4 additions & 2 deletions src/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

#include "v8.h"

#include "node.h"

#include <climits>
#include <cstddef>
#include <cstdio>
Expand Down Expand Up @@ -110,8 +112,8 @@ struct AssertionInfo {
const char* message;
const char* function;
};
[[noreturn]] void Assert(const AssertionInfo& info);
[[noreturn]] void Abort();
[[noreturn]] void NODE_EXTERN_PRIVATE Assert(const AssertionInfo& info);
[[noreturn]] void NODE_EXTERN_PRIVATE Abort();
void DumpBacktrace(FILE* fp);

// Windows 8+ does not like abort() in Release mode
Expand Down
Loading