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 mount functionality to 'Directory' #177

Conversation

stagnation
Copy link
Contributor

This is needed for 'chroot' runners that must mount the special filesystems '/proc' and '/sys' in the input root.
buildbarn/bb-remote-execution#115

Copy link
Contributor Author

@stagnation stagnation left a comment

Choose a reason for hiding this comment

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

FTR: I can not request changes on my own PR.

pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/mount_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/mount_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/BUILD.bazel Outdated Show resolved Hide resolved
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from ac05320 to 03950d3 Compare September 6, 2023 09:20
pkg/filesystem/directory.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/mount_unix.go Outdated Show resolved Hide resolved
_p1 = nil
}

r0, _, e1 := unix.Syscall6(
Copy link
Member

Choose a reason for hiding this comment

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

The same holds for fsconfig(). We could also let golang.org/x/sys/unix ship with a binding for that. My experience is that the folks maintaining that package are willing to accept such changes. They also review them in a timely manner.

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 have updated the commit with the patch file, and will wait a couple more days for them to see my comment on the original review before forking it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a new patch, with the new proposed API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There has not been any progress on the review since we last discussed the patch :/
golang/go#59537 (comment)

pkg/filesystem/mount_unix.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_unix.go Outdated Show resolved Hide resolved
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 03950d3 to b29adf9 Compare September 6, 2023 13:33
pkg/filesystem/local_directory_linux.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_linux.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_linux.go Outdated Show resolved Hide resolved
Copy link
Member

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

👍

@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 772956f to 4269bbc Compare September 15, 2023 08:13
pkg/filesystem/directory.go Outdated Show resolved Hide resolved
go_dependencies.bzl Outdated Show resolved Hide resolved
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch 3 times, most recently from 58135c0 to 02a95a4 Compare October 2, 2023 08:52
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 02a95a4 to 0af1b16 Compare November 10, 2023 13:39
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 0af1b16 to c822402 Compare November 30, 2023 08:08
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from c822402 to 857e0a7 Compare December 8, 2023 12:40
pkg/filesystem/local_directory_linux.go Outdated Show resolved Hide resolved
pkg/filesystem/local_directory_linux.go Show resolved Hide resolved
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 857e0a7 to 646f697 Compare December 13, 2023 10:51
@stagnation
Copy link
Contributor Author

Applied the last review comments :)

Copy link
Member

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

Looks good! Will merge when CI is happy.

This is needed for 'chroot' runners that must mount the special
filesystems '/proc' and '/sys' in the input root.
The necessary syscalls are not yet merged into the 'sys' package,
so this includes a patch while we work to upstream the code.
@stagnation stagnation force-pushed the feature/mount-special-filesystems-in-chroot-runners branch from 646f697 to 7aef0b0 Compare December 20, 2023 10:14
@EdSchouten EdSchouten merged commit 18fd05d into buildbarn:master Dec 20, 2023
1 check passed
@stagnation stagnation deleted the feature/mount-special-filesystems-in-chroot-runners branch December 20, 2023 12:23
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.

2 participants