changes required for VFS API in C# by kivikakk · Pull Request #53 · github/libprojfs

kivikakk · 2019-02-25T05:13:03Z

With these changes, the VFS API in C# (at ~~github/VFSForGit#2~~ microsoft/VFSForGit#848) can be used.

pid_t and int should both be 4 bytes on most systems, but one's more obvious than the other.

kivikakk · 2019-02-26T23:18:08Z

@chrisd8088 fyi, this now has the part of work I talked about earlier (viz. 453409f, which removes VFS API-related things). Tests all pass and I think I did it right, but a once-over from you wouldn't go amiss, esp. in the testing machinery.

chrisd8088

So lovely to see this shrinking things down and making such a clean line between the two projects. Thank you, @kivikakk!

chrisd8088 · 2019-02-27T22:39:30Z

include/config.h.in~

@@ -0,0 +1,103 @@
+/* include/config.h.in.  Generated from configure.ac by autoheader.  */


I think this file was included as an addition in the commit by accident.

(As a sidebar, I initially considered putting *~ into .gitignore because I'm a vim user and it sometimes leaves these backup files lying around. But then somewhere I think I read advice to not put in your personal stuff in there, so I left it out. However, since the Linux kernel happily puts *~ in theirs, I think we'd be in good company if we wanted to add it. :-)

Very much so! I don't have such an ignore rule because I always configure my editors not to leave that junk lying around -- I think some part of autotools is producing this particular one, since it keeps coming back.

(I have a ~/.gitignore where I put stuff like .*.sw? (because vim) -- set excludesfile under [core] in your ~/.gitconfig to /home/xyzzy/.gitignore and it'll apply all the time!)

Yeah, looking at my own ~/.gitignore I see I've already got it ignoring Vim's .*.swp backups, so this must indeed come out of autotools. Sigh.

chrisd8088 · 2019-02-27T23:30:25Z

t/Makefile.am

-	 t703-vfs-event-null.t \
-	 t704-vfs-event-allow.t
-else !ENABLE_VFSAPI
-SKIP_TESTS = t[5-9]??


Because we don't need to skip any VFS-related tests when --enable-vfs-api is not given, I'm pretty sure there's a line below this which can be removed (and the indentation changed), from:

PROJFS_SKIP_TESTS="$(SKIP_TESTS) $$PROJFS_SKIP_TESTS" \ $(PROVE) --exec $(SHELL) \ $(PROJFS_PROVE_OPTS) $$prove_dir; \

to:

$(PROVE) --exec $(SHELL) \ $(PROJFS_PROVE_OPTS) $$prove_dir; \

Ah, thanks!

chrisd8088 · 2019-02-28T00:04:59Z

lib/projfs.c

+		ssize_t res = write(fd, bytes, count);
+		if (res == -1)
+			return errno;
+		bytes = (void *)(((uintptr_t)bytes) + res);


Normal precedence should, I think, let you just do (void *)((uintptr_t)bytes + res), but I'm also wondering if it's slightly more readable to use (char*)bytes + res, which should be equivalent since sizeof(char) == 1 in C.

Just watched s2.e5 Saints of Imperfection! Major spoilers!

chrisd8088 · 2019-02-28T00:06:09Z

configure.ac

 AC_TYPE_SIZE_T
 AC_TYPE_UINT64_T

-# TODO: for VFS API but can't be conditional; remove if VFS API migrated


Yay! Lovely to see this gone! 🙇

chrisd8088 · 2019-02-28T00:07:13Z

docs/build-install.md

-To build libprojfs with the VFSForGit API option (which is
-required if you plan to run a VFSForGit "provider" such as MirrorProvider,
-the test provider we are developing against for now), add the
-`--enable-vfs-api` flag to the arguments for `configure`:


Things are definitely getting tidier! 😄

chrisd8088 · 2019-02-28T00:12:49Z

t/test-lib-event.sh

-			event_msg_head="$event_msg_head, $4"
-		fi
-		event_msg_head="$event_msg_head: $code, "
-		event_msg_tail=""


So much simpler and nicer!

include/projfs.h

See discussion at #53 (comment).

Recommendations accepted!

Without the VFS API code, there is no longer a need to customize the regular expression used to match symbol names for export in our libtool invocation.

chrisd8088 · 2019-03-01T22:58:35Z

Hey @kivikakk -- I added a couple of cleanups to this branch which fall out of the VFS API removal. Hope they look OK!

And I also made a follow-on PR #54 which I think can be merged into this one, which simplifies the library by removing the projfs_i.h internal header file.

And there's a now further PR #55 which should first be merged into #54, if it looks OK to you!

During the initial work on projection and while we had the VFS API in this library, I envisioned a need for the VFS code to call an internal, common function to make a directory (either projected, or normal). That need never materialized, so we can eliminate this function entirely.

Without any shared code between the primary library and a VFS API, there's no need for an internal header file at the moment, so we can eliminate it.

As there is no compelling reason not to allow libprojfs callers to specify the permission mode bits of a projected directory, we can support this in our API, and defer to our callers (VFSForGit in particular) to supply the directory permissions they expect or require.

allow arbitrary permission modes on projected dirs

kivikakk · 2019-03-04T01:18:41Z

Nice! All looking good to me 😻

Remove internal header file

kivikakk added 2 commits February 25, 2019 14:56

specify int instead of pid_t

2c8218a

pid_t and int should both be 4 bytes on most systems, but one's more obvious than the other.

add projfs_write_file_contents

55001ac

kivikakk mentioned this pull request Feb 25, 2019

VFS API github/VFSForGit#2

Closed

kivikakk added 2 commits February 26, 2019 12:23

use vfs-api branch of VFSForGit

9247356

vfs-api branch as of upstream PR

d99dd5a

kivikakk mentioned this pull request Feb 26, 2019

[Linux] call projfs directly, don't use intermediary projfs_vfsapi microsoft/VFSForGit#848

Merged

chrisd8088 added api libprojfs API vfs VFSForGit API labels Feb 26, 2019

remove VFS API and supporting components

453409f

chrisd8088 previously requested changes Feb 28, 2019

View reviewed changes

kivikakk added 3 commits February 28, 2019 12:11

remove inadvertent backup file

2fd9428

remove some unused cruft, clean up casts

d98403c

use pid_t again

e8e20b3

See discussion at #53 (comment).

chrisd8088 added 2 commits March 1, 2019 14:21

simplify naming of test programs after VFS removal

da7b9ce

simplify global symbol library export regex

ee76d36

Without the VFS API code, there is no longer a need to customize the regular expression used to match symbol names for export in our libtool invocation.

chrisd8088 and others added 4 commits March 1, 2019 22:20

remove unnecessary internal header file

0b028b7

Without any shared code between the primary library and a VFS API, there's no need for an internal header file at the moment, so we can eliminate it.

Merge pull request #55 from github/remove-vfsapi-add-dirmode

04e09cf

allow arbitrary permission modes on projected dirs

Merge pull request #54 from github/remove-vfsapi-internal-header

9f09eb7

Remove internal header file

kivikakk merged commit 272e5bb into master Mar 4, 2019

kivikakk deleted the remove-vfsapi branch March 4, 2019 01:57

chrisd8088 mentioned this pull request Mar 5, 2019

remove support files formerly used to test VFS API #57

Merged

chrisd8088 mentioned this pull request Mar 28, 2019

Migrate VFS API support into VFSForGit repo #48

Closed

		@@ -0,0 +1,103 @@
		/* include/config.h.in. Generated from configure.ac by autoheader. */

Conversation

kivikakk commented Feb 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kivikakk commented Feb 26, 2019

Uh oh!

chrisd8088 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chrisd8088 commented Mar 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kivikakk commented Mar 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kivikakk commented Feb 25, 2019 •

edited

Loading

chrisd8088 commented Mar 1, 2019 •

edited

Loading