Dokan_fuse build system enhancements #337

Rondom · 2016-09-14T14:32:15Z

This branch contains changes that unify the directory structure of dokan_fuse. This enables compilation of the same code base against either dokanfuse or libfuse without changes.
We are also adding an install target so one can "install" dokanfuse into a Unix-directory hierarchy (Cygwin, MSYS, MingW-crosscompile under Linux) when compiling oneself.

In addition to that this branch contains commits that make build scripts return a non-zero error code in case one of the build-commands fails.

This work has been sponsored by Intel Corporation.

Rondom · 2016-09-14T14:39:34Z

@jetwhiz, can you give this a try once Appveyor has finished the compilation? The fuse-mirror-compilation actually tests this, but given that we are so close before 1.0.0 it would be nice to have another human-being taking a second look at it.

One should be able to compile against libfuse or dokanfuse only by changing the include path and linker-path, i.e.

-I ${dokan_install_dir}/include -L ${dokan_install_dir}/

Rondom · 2016-09-14T15:00:13Z

Also everyone else is welcome to review the individual commits and test the build, of course. Especially the installer might need attention, because that is an area I am not familiar with.

I don't want to be the one that introduces a regression so close before 1.0.0 :-D

jetwhiz · 2016-09-14T18:29:18Z

Hi @Rondom -- I'll give it a shot! Can I build this using Visual Studio, or does this need to be done using Cygwin for the test?

Rondom · 2016-09-14T23:47:38Z

Compiler does not matter as long as you link to the right library (dokanfuse.dll for MSVC, libdokanfuse.dll for MinGW, cygdokanfuse.dll for CygWin).
Setting the include path to [Dokan-Installation-Folder]/include and the library search path to the installation folder itself should work.

Rondom · 2016-09-15T10:16:02Z

@jetwhiz I messed up on the installer. 😳 It should be fixed now. Please download again.

Liryna

@Rondom you removed dokanfuse.h, fusemain.h, scopehuard.h and utils.
For you they are not needed by someone wanting to link against the library ?

Rondom · 2016-09-15T12:13:02Z

I thought they were C++ headers internal to dokan_fuse. Are they considered part of the API? If they are, they should be there of course.

Oh, and can someone enlighten me what this ScopeGuard C++-template-craziness does? 😝

jetwhiz · 2016-09-16T15:44:58Z

@Rondom -- I'm not sure if I'm doing something wrong, but when I replace the system32 dll files with the ones built from this branch, encfs4win dies.

Dokan: debug mode on
Dokan: use stderr
AllocationUnitSize: 512 SectorSize: 512
Dokan Error: CreatFile Failed \\.\Dokan_1: 2
send global release for Z:
Dokan Error: Failed to open \\.\Dokan_1 with code 2
Failed to unmount: Z:

Rondom · 2016-09-16T15:53:13Z

Updating the driver as well should fix this. This was unfortunately necessary because of #320.

Rondom · 2016-09-16T16:19:49Z

@Liryna Indeed some of the stuff might be useful, other stuff looks more internal, especially the stuff relying on C++. Does someone have an idea or suggestions, about what is useful and what is internal?

jetwhiz · 2016-09-16T16:22:54Z

Unfortunately I'm not able to build the drivers in the project, since I don't have enough disk space to install the WDK 10. Do you have an already-built one for Windows 7 x86 prepared? Or maybe you could provide me with the branch's Dokany installer and I can install everything fresh.

Rondom · 2016-09-16T16:58:41Z

Oh! No need to build yourself.

Appveyor is building binaries for every pull request and push.
The drivers are signed with a self-signed certificate so you have to import that certificate and enable test mode. See the wiki for information. The PowerShell-script mentioned there does not work on Win7. I have filed #342 with more information about this. Taking CertMgr.exe (a few KB) from another machine with Windows SDK or VS installed, works fine.

You can easily reach the Appveyor-builds by clicking the green ✓ next to the latest commit in the Pull-Request. Link to the artifacts of the at the time of this writing latest build of this PR

Liryna · 2016-09-18T16:36:52Z

@Rondom I have been able to build fusexmp.c on cygwin with your install package without issue 👍

There is only utils.h that should be added in the installer. It has some helper that provides the library and it is used by some projects (@jetwhiz does use it if I remember).
https://github.com/dokan-dev/dokany/blob/master/dokan_fuse/src/dokanfuse.def#L85-L96

Indeed some of the stuff might be useful, other stuff looks more internal, especially the stuff relying on C++. Does someone have an idea or suggestions, about what is useful and what is internal?

You know what ? I removed ScopeGuard.h, rebuild, and nothing happened 😄
It seems to come from Loki http://loki-lib.sourceforge.net/html/a00667.html
This probably was used a looonnng time ago in dokanfuse.cpp and not removed from the solution.
You can remove it in your pull request if you want 👍

Good job @Rondom 👍 and big ❤️ to the make install

(there is a couple of warnings showed with https://github.com/dokan-dev/dokany/pull/337/files#diff-e2cb466c3a4e3af410a8719f1f2b0340R10, do you plan to fix it ? otherwise I will do it after)

jetwhiz · 2016-09-18T18:15:58Z

@Rondom -- I did a bit of testing using the PR artifact installer and didn't notice any problems that stood out on my end when running encfs4win.

Regarding the PowerShell-script, unfortunately I don't have access to a Win 7 build machine -- I only do bare-bones testing in a pristine Win 7 VM. Does using CertMgr not work when Import-Certificate isn't detected?

Rondom · 2016-09-18T18:18:09Z

@Liryna

There is only utils.h that should be added in the installer. It has some helper that provides the library and it is used by some projects (@jetwhiz does use it if I remember).

Ok, I will add back utils.h to the installation. This is a C++ header, shall I #ifdef things and disable symbol-mangling for the C-parts, so it can be used both in C++ and C (minus the C++ parts)?

I will also delete ScopeGuard.h. I do not have the energy to find out myself, but I am still curious about what it does. ;-)

(there is a couple of warnings showed with https://github.com/dokan-dev/dokany/pull/337/files#diff-e2cb466c3a4e3af410a8719f1f2b0340R10, do you plan to fix it ? otherwise I will do it after)

I intentionally enabled the warnings. Sometimes warnings point out things one is not aware of that should be fixed. If there are warnings that cannot be fixed, we can always disable individual warnings.
I have a patch for the DWORDs/printf-format-specifiers warning ready, that I will include in the update of this pull request tomorrow. The rest of the warnings I have not yet looked into and I do not have the time right now. So if you want to look into them please go ahead! 👍

Rondom · 2016-09-18T18:19:23Z

Regarding the PowerShell-script, unfortunately I don't have access to a Win 7 build machine -- I only do bare-bones testing in a pristine Win 7 VM. Does using CertMgr not work when Import-Certificate isn't detected?

@jetwhiz CertMgr.exe DOES work (see the wiki-article a few lines above). The only "problem" is that you need to download VS or Windows SDK to get these few kilobytes of an EXE which is a bit overkill. Copying over CertMgr from another machine with VS, works like a charm and so does using the GUI. :-D

Rondom · 2016-09-18T18:23:56Z

@Liryna I will also remove

dokany/dokan_fuse/include/utils.h

Lines 8 to 12 in 4d11614

    
           /*#ifdef _MSC_VER 
        
           #define DLLLOCAL 
        
           #else 
        
           #define DLLLOCAL __attribute__ ((visibility("hidden"))) 
        
           #endif*/

The code is commented and not used anywhere as far as I can see.

Liryna · 2016-09-18T18:51:34Z

Ok, I will add back utils.h to the installation. This is a C++ header, shall I #ifdef things and disable symbol-mangling for the C-parts, so it can be used both in C++ and C (minus the C++ parts)?

Yes this would be great!

Liryna · 2016-09-19T19:52:11Z

@Rondom I will provide you a patch for the build warnings tomorrow morning.
Can you make the last changes also at this time ?

I would like to make the release tomorrow.

Rondom · 2016-09-20T00:20:26Z

First thing I will do in the morning

Liryna · 2016-09-20T07:45:19Z

@Rondom here is the patch:
0001-Fix-fuse-build-warnings.txt

If one build-command fails the other build commands wil still be run, but the script will exit with a non-zero errorcode once all commands are finished.

We still default to Release for CMAKE_RELEASE_TYPE, but if another value is specified on command line, we honour that.

This adds an install target that installs the headers in the same directory structure as libfuse. This makes it easy to support compiling against either libfuse or libdokanfuse1 from the same codebase. This fixes dokan-dev#250 for manual compilations under Cygwin or for MingW.

This is a rework of the dokan_fuse build-script: * out-of-tree build to avoid fragile deleting of files * use install target for "installation" into target-dir * fix MinGW output by calling bash directly, this makes the output visible on command-line. Closes dokan-dev#333. * return non-zero error code in case any of the commands fails

Installs the public FUSE headers in a directory structure that resembles that of Linux FUSE. Linux FUSE requires a fuse.h in the top-level include-directory. Closes dokan-dev#250.

DWORD and ULONG are defined as unsigned long int on Windows. Cygwin64 is LP64. Thus it defines them as unsigned int (to keep them 32-bits wide). In order to enable portable printing, we define macros modeled after inttypes.h like for example PRIuDWORD or PRIxDWORD. Using these macros ensures that the correct printf-format-specifier is used depending on the platform.

The file was not used anywhere in the code. So it is safe to delete it.

Disable symbol-mangling for the C-parts of utils.h to enable using some utility functions from C as well.

Rondom · 2016-09-20T12:42:59Z

What I have tested:

Linking against cmake-installed FUSE's unixTimeToFiletime from utils.h using a GCC (G++ and MSVC(++) untested)
Presence of utils.h in Dokan-installation-folder under Win64
No warnings under MingW(32|64) or Cygwin(32|64)
Short test against my MingW32-based filesystem (debug prints still working as before)
version numbers of (cyg|lib)*dokanfuse.dll, dokan1.sys, dokan1.dll, DokanSetup.exe

Not tested:

FUSE-Mirror debug-prints on Cygwin(32|64)
Presence of utils.h in Dokan-installation-folder under Win32
Linking against functions in utils.h using a C++ compiler (should be tested by dokan_fuse compilation itself)

Liryna · 2016-09-20T12:48:58Z

Thank you @Rondom for making this and in time 😃 !

👍 Let's the release begin !

Rondom · 2016-09-20T12:59:48Z

One nitpick, can you (manually! don't delay the release further) add the version number to each file's filenames when you do the release? I think it is nicer that way because it keeps people's download-folder nice and tidy. i.e. DokanSetup-1.0.0.5000.exe and so on.

Liryna · 2016-09-20T13:00:28Z

@Rondom Alright I will do this 👍

Rondom · 2016-09-20T13:15:42Z

Make sure this is mentioned in the changelog

Liryna · 2016-09-20T13:18:00Z

👍 I will do it

Currently I have issue to build fuse on my windows 😢

      0 [main] bash 8436 child_info_fork::abort: C:\cygwin\bin\cygiconv-2.dll: Loaded to different address: parent(0xF50000) != child(0xEA0000)
/usr/bin/bash: fork: retry: No child processes
      0 [main] bash 8020 child_info_fork::abort: C:\cygwin\bin\cygiconv-2.dll: Loaded to different address: parent(0xF50000) != child(0xF40000)
/usr/bin/bash: fork: retry: No child processes
      0 [main] bash 7788 child_info_fork::abort: C:\cygwin\bin\cygiconv-2.dll: Loaded to different address: parent(0xF50000) != child(0xE60000)
/usr/bin/bash: fork: retry: No child processes
      0 [main] bash 10652 child_info_fork::abort: C:\cygwin\bin\cygiconv-2.dll: Loaded to different address: parent(0xF50000) != child(0xFF0000)
/usr/bin/bash: fork: retry: No child processes
      0 [main] bash 9168 child_info_fork::abort: C:\cygwin\bin\cygiconv-2.dll: Loaded to different address: parent(0xF50000) != child(0xE80000)
/usr/bin/bash: fork: Resource temporarily unavailable

did you face the issue ?

If I run C:\cygwin\bin\bash outside the script it work 😢

Rondom · 2016-09-20T13:20:11Z

Blind guess: Do you have multiple versions of Cygwin running? Did you upgrade today? Try a reboot to make sure no old stuff is loaded.

Liryna · 2016-09-20T13:37:36Z

@Rondom cde6cd5 is that alright for you ?

Rondom · 2016-09-20T13:38:18Z

Yes

Rondom force-pushed the build-system-enh-rebase branch from 71adc37 to 543a4b5 Compare September 15, 2016 09:54

Liryna reviewed Sep 15, 2016

View reviewed changes

Rondom added 8 commits September 20, 2016 13:38

Add error-detection for build.bat

3a5f45e

If one build-command fails the other build commands wil still be run, but the script will exit with a non-zero errorcode once all commands are finished.

dokan_fuse: Honour CMAKE_RELEASE_TYPE

6d51499

We still default to Release for CMAKE_RELEASE_TYPE, but if another value is specified on command line, we honour that.

dokan_fuse: Compile and link version resource

4b8430a

dokan_fuse: Remove execute-bit from dokanfuse.cpp

6c35385

dokan_wix: Use FUSE-compatible dir-hierarchy

20caf59

Installs the public FUSE headers in a directory structure that resembles that of Linux FUSE. Linux FUSE requires a fuse.h in the top-level include-directory. Closes dokan-dev#250.

Adrien JUND and others added 3 commits September 20, 2016 13:38

dokan_fuse: Fix build warnings

74f2526

dokan_fuse: Delete unused ScopeGuard.h

3af3c96

The file was not used anywhere in the code. So it is safe to delete it.

dokan_fuse: Remove uncommented code in utils.h

bddd734

Rondom force-pushed the build-system-enh-rebase branch from 543a4b5 to 038c0d4 Compare September 20, 2016 12:03

dokan_fuse: Make utils.h usable under C

a804e20

Disable symbol-mangling for the C-parts of utils.h to enable using some utility functions from C as well.

Liryna merged commit 66a1e2b into dokan-dev:master Sep 20, 2016

Rondom deleted the build-system-enh-rebase branch October 28, 2016 00:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dokan_fuse build system enhancements #337

Dokan_fuse build system enhancements #337

Rondom commented Sep 14, 2016 •

edited

Rondom commented Sep 14, 2016

Rondom commented Sep 14, 2016

jetwhiz commented Sep 14, 2016

Rondom commented Sep 14, 2016 •

edited

Rondom commented Sep 15, 2016

Liryna left a comment

Rondom commented Sep 15, 2016 •

edited

jetwhiz commented Sep 16, 2016 •

edited

Rondom commented Sep 16, 2016

Rondom commented Sep 16, 2016

jetwhiz commented Sep 16, 2016

Rondom commented Sep 16, 2016 •

edited

Liryna commented Sep 18, 2016

jetwhiz commented Sep 18, 2016 •

edited

Rondom commented Sep 18, 2016 •

edited

Rondom commented Sep 18, 2016 •

edited

Rondom commented Sep 18, 2016

Liryna commented Sep 18, 2016

Liryna commented Sep 19, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016 •

edited

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Dokan_fuse build system enhancements #337

Dokan_fuse build system enhancements #337

Conversation

Rondom commented Sep 14, 2016 • edited

Rondom commented Sep 14, 2016

Rondom commented Sep 14, 2016

jetwhiz commented Sep 14, 2016

Rondom commented Sep 14, 2016 • edited

Rondom commented Sep 15, 2016

Liryna left a comment

Choose a reason for hiding this comment

Rondom commented Sep 15, 2016 • edited

jetwhiz commented Sep 16, 2016 • edited

Rondom commented Sep 16, 2016

Rondom commented Sep 16, 2016

jetwhiz commented Sep 16, 2016

Rondom commented Sep 16, 2016 • edited

Liryna commented Sep 18, 2016

jetwhiz commented Sep 18, 2016 • edited

Rondom commented Sep 18, 2016 • edited

Rondom commented Sep 18, 2016 • edited

Rondom commented Sep 18, 2016

Liryna commented Sep 18, 2016

Liryna commented Sep 19, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016 • edited

Liryna commented Sep 20, 2016

Rondom commented Sep 20, 2016

Rondom commented Sep 14, 2016 •

edited

Rondom commented Sep 14, 2016 •

edited

Rondom commented Sep 15, 2016 •

edited

jetwhiz commented Sep 16, 2016 •

edited

Rondom commented Sep 16, 2016 •

edited

jetwhiz commented Sep 18, 2016 •

edited

Rondom commented Sep 18, 2016 •

edited

Rondom commented Sep 18, 2016 •

edited

Rondom commented Sep 20, 2016 •

edited