Skip to content

Implement resolve-system-dependencies in C++#1030

Merged
edolstra merged 11 commits intoNixOS:masterfrom
pikajude:rsd-cc
Aug 31, 2016
Merged

Implement resolve-system-dependencies in C++#1030
edolstra merged 11 commits intoNixOS:masterfrom
pikajude:rsd-cc

Conversation

@pikajude
Copy link
Copy Markdown
Contributor

Instead of serializing a hashmap to a file as Perl does (which would require an external library dependency), this script stores the dependency list for each lib separately in a cache directory. This should be faster than the old implementation because no resolving is done after the first time the script runs.

Accepting code review as my C++ is not very clean.

@domenkozar domenkozar added this to the perl-to-c++ milestone Aug 15, 2016
static auto cacheDir = Path{};

Path resolveCacheFile(const Path & lib) {
Path lib2 = Path(lib);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of using a const reference parameter and doing explicit copying, you could directly accept the lib argument via value.

@copumpkin
Copy link
Copy Markdown
Member

At the highest level, I'd suggest using mmap because it saves you from doing 90% of the nasty seeking/reading/sizeof-ing. You map the object into memory and then the various structs already point at the correct data. That's how dyld does it, and I think it'll make your life a lot simpler.

@copumpkin
Copy link
Copy Markdown
Member

(not that I'd block merging this because you didn't use mmap, but I think it'd be a bit more pleasant)

dylc.dylib.name.offset - (sizeof(uint32_t) * 2) + pos,
SEEK_SET);
fread(dylib_name, sizeof(char), cmd.cmdsize, obj_file);
fseek(obj_file, pos, SEEK_SET);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This lacks error checking. I would use C++ <iostream>, or read the whole file into memory using readFile() for simplicity.

@domenkozar
Copy link
Copy Markdown
Member

@pikajude does this address #941?

@domenkozar
Copy link
Copy Markdown
Member

Also relevant is #786

@edolstra edolstra mentioned this pull request Aug 16, 2016
12 tasks
@pikajude
Copy link
Copy Markdown
Contributor Author

pikajude commented Aug 16, 2016

@domenkozar #941 will no longer apply because otool is not used or even required in this rewrite (the mach-o files are directly read by the script instead). #786 looks to be a fundamental problem with pre-build hooks in general, and as such will require more investigation.

@edolstra @copumpkin I have rewritten the script to use mmap, meaning there should be a little less fiddling around with FILE*s.

@edolstra edolstra merged commit aa1ea0d into NixOS:master Aug 31, 2016
Ma27 pushed a commit to Ma27/nix that referenced this pull request Mar 8, 2026
the cgroups experimental feature does not work properly without this
because we do not stop subdaemons when the main daemon is shut down.
systemd needs the assigned cgroups to be empty to restart the daemon
and thus cannot cleanly restart the daemon if any connections exist.
starting a fresh unit for each connection creates a new cgroup every
time instead of sharing any delegations and thus solves the problem.

fixes NixOS#1030

Change-Id: Id6c458aad30eaa08c3609ac8280a7dde8e8f3cf9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants