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

dokan_fuse: Add libfuse-compatible pkg-config #375

Merged
merged 2 commits into from
Nov 5, 2016

Conversation

Rondom
Copy link
Contributor

@Rondom Rondom commented Oct 25, 2016

Don't merge yet. This needs further testing and input.
This fixes #338.

This generates a pkg-config-file that can be used as a drop-in "replacement" for libfuse, i.e.

$ pkg-config --libs --cflags fuse

will return the dokanfuse-flags.

Required testing

I have only tested this on Debian Stretch. Thus, it is lacking testing on Windows. I would be happy if someone could test it on

  • Cygwin
  • MSYS2-Installer - MinGW-W64-32bit
  • MSYS2-Installer - MinGW-W64-64bit

I currently do not have any working Windows-system at hand to test myself.

You can add -DCMAKE_INSTALL_PREFIX="/tmp/usr/ to the cmake-invokation to define another prefix to avoid cluttering your global directories while testing. You can then use the PKG_CONFIG_PATH environment-variable to point pkg-config to the right lib/pkgconfig-dir.
Situations to test:

  • Normal installation (cd build && cmake .. && make install)
  • Custom absolute path for LIBDIR and and INCLUDEDIR (i.e. -DCMAKE_INSTALL_LIBDIR=/some/absolute/path/that/is/different and -DCMAKE_INSTALL_INCLUDEDIR=/some/absolute/path/that/is/different).

I am a bit confused about whether we need to use LIBDIR or BINDIR, because I was unfamiliar with the Windows-only concept of "import libraries" (research shows that it had historic reasons to support compiling for OS/2 from NT without having the OS/2 DLLs).

Discussion

The pkg-config-file will be installed as fuse.pc. So we kind of snatch the libfuse name. This could be considered problematic for users that prefer to have different fuse-libaries installed. I have some suggestions for that

  1. Leave as it is, always install a fuse.pc
  2. Add some option to cmake that allows one to disable the installation of fuse.pc ('-DINSTALL_PKG_CONFIG=OFF`)'
  3. Install a second pkg-config-file dokanfuse.pc and add an option like -DINSTALL_PKG_CONFIG=(both|fuse|dokanfuse|none), default would be both

@Liryna
Copy link
Member

Liryna commented Oct 25, 2016

Hi @Rondom ,

Ah ❤️ !!!

I am absolutely not familiar with this but I can test 👍 on windows.

About package name, I think the third option would be the best ! This will let choice for the users.

Great job @Rondom !

@Rondom
Copy link
Contributor Author

Rondom commented Oct 25, 2016

Then wait with the testing for now. On a second thought, I can also use it while building the FUSE-mirror, the comments in the code even suggest to get the CFLAGS and LDFLAGS using the pkg-config....

gcc -Wall fusexmp.c $(pkg-config fuse --cflags --libs) -o fusexmp

@Rondom
Copy link
Contributor Author

Rondom commented Oct 25, 2016

About option 3: I thought about this more and noticed that we would need the correct Dokan version for a dokanfuse.pc. So this is dependent on some other work I planned to be doing (refactor how the version number gets embedded in the build). This might take a bit more time (which I do not have right now, I am afraid.

So, for this PR we have two possibilities: Wait until we can do it perfectly (retrieve the version) or simply implement option 2: "Add some option to cmake that allows one to disable the installation of fuse.pc (-DINSTALL_FUSE_PKG_CONFIG=OFF) and merge this PR for now. A dokanfuse.pc with the version embedded can come in a later pull-request.

What do you think?

@Liryna
Copy link
Member

Liryna commented Oct 25, 2016

No problem, second option is also great 👍

Generate a pkg-config-file that can be used as a drop-in "replacement"
for libfuse, i.e.
$ pkg-config --libs --cflags fuse
will return the dokanfuse-flags.

Closes dokan-dev#338.
@Rondom
Copy link
Contributor Author

Rondom commented Oct 27, 2016

Now modified the build-script to use pkg-config.
Oh, and the PR cannot be merged right now anyway, I guess. We rather want this to be in 1.1.0 and not in 1.0.1. :-D

@Liryna
Copy link
Member

Liryna commented Oct 27, 2016

Thank you @Rondom ! We will wait 1.1.0 to merge this so ❤️ (and test it on windows at this moment)

@Liryna
Copy link
Member

Liryna commented Nov 5, 2016

@Rondom shoud we merge this 👍 ?

@Rondom
Copy link
Contributor Author

Rondom commented Nov 5, 2016

Sure

@Liryna Liryna merged commit bd86267 into dokan-dev:master Nov 5, 2016
@Liryna
Copy link
Member

Liryna commented Nov 5, 2016

Thank you @Rondom 😍 !

@Rondom Rondom deleted the pkg-config branch November 5, 2016 16:12
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.

dokan_fuse: Ship pkg-config file
2 participants