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

DAOS-623 build: Create a simple RPM package #1

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

jolivier23
Copy link

@jolivier23 jolivier23 commented Feb 23, 2024

Makes simple set of packages for fused dynamic libraries.
Renames the library and include directory from fuse3 to fused to
avoid conflicts with upstream packages.

Makes the simplest of RPM packages for fused.
Renames the static library and include dir from
fuse3 to fused to avoid any conflict with
standard libfuse3.a.

Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Run-GHA: true

Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
@jolivier23
Copy link
Author

@brianjmurrell can I pick your brain on this one?

But before I do so, here are the reasons for doing a fork of fuse

  1. the upstream libfuse releases at too slow a rate
  2. FUSE provides terrrible performance out of the box and we want to experiment and possibly ship a custom kernel module. As such, we would want to customize libfuse as well. We could do the latter via patches but I wanted to have something that would not mess up other fuse file systems executing on the same node.

My builder renames fuse3 to fused so the libraries provided by the fused package won't conflict with libraries installed by the upstream library.

I can build both debian and rpm packages locally, I'm just trying to get the basic RPM/deb build working in Jenkins. For this purpose, I used the raft module as an example since it also is a fork where the code resides in the same repository. But it doesn't seem to be working for various reasons and without any visibility into Jenkins, I'm finding my ability to fix it limited.

I wanted to not have to bug you at all til I had it working but failed.

Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
Signed-off-by: Jeff Olivier <[email protected]>
@jolivier23 jolivier23 requested review from ashleypittman and a team June 4, 2024 21:10
@brianjmurrell
Copy link

Do you have a reference as to how/where this package is used?

Copy link

@brianjmurrell brianjmurrell left a comment

Choose a reason for hiding this comment

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

Just -1 until the purpose and use is more clearly understood.

@jolivier23
Copy link
Author

jolivier23 commented Jun 5, 2024

Do you have a reference as to how/where this package is used?

Yeah, the idea is to move our fuse in this direction for a couple of reasons

  1. there are other users of fuse so building fuse that everyone else uses seems risky. I actually ran into issues on my cloudtop with inability to mount internal stuff when I tried using our build of fuse.
  2. This renames all of the libraries and header locations so that it won't conflict with default fuse
  3. Gives us the ability to maintain our own fork with patches that upstream may not take that are specific to daos.

Here is a proof of concept patch to DAOS
daos-stack/daos#14077

That patch can't be landed until we land this and then figure out proper versioning.

Copy link

@brianjmurrell brianjmurrell left a comment

Choose a reason for hiding this comment

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

You have rpmlint errors/warning that need to be resolved for a distributed package.

You also don't really want githash info included in the Release if you are building from a tag:

$ rpm -q --provides fused-1.0.0-1.2014.g0303197.el8.x86_64.rpm 
fused = 1.0.0-1.2014.g0303197.el8
fused(x86-64) = 1.0.0-1.2014.g0303197.el8

These should look like:

$ rpm -q --provides fused-1.0.0-1.el8.x86_64.rpm 
fused = 1.0.0-1.el8
fused(x86-64) = 1.0.0-1.el8

@@ -14,7 +14,6 @@
*.orig
*~
Makefile.in
Makefile

Choose a reason for hiding this comment

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

Why don't we want to ignore this file? It's autogenerated from Makefile.in isn't it?

@@ -0,0 +1,43 @@
#!/usr/bin/groovy
/* Copyright (C) 2019 Intel Corporation

Choose a reason for hiding this comment

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

Copyright update here?

Comment on lines +19 to +20
find lib -type f -name "*" -exec sed -i 's/fuse3/fused/g' {} ';'
find include -type f -name "*" -exec sed -i 's/fuse3/fused/g' {} ';'

Choose a reason for hiding this comment

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

Is the -name * redundant? Apart from that, this would be much more efficient:

Suggested change
find lib -type f -name "*" -exec sed -i 's/fuse3/fused/g' {} ';'
find include -type f -name "*" -exec sed -i 's/fuse3/fused/g' {} ';'
find lib -type f -name "*" -print0 | xargs -0r sed -i 's/fuse3/fused/g' {} ';'
find include -type f -name "*" -print0 | xargs -0r sed -i 's/fuse3/fused/g' {} ';'

as it only runs sed as many times is as necessary to fit within command-line length limits and passes as many filenames to a single sed command that fits within the limit.

But that said (heh, see what I did there?) isn't this kind of substitution something that should be done in the libfuse fork so that people building that from source don't also have to know to do this s///?

Comment on lines +21 to +22
sed -i 's/fuse3/fused/g' meson.build
sed -i 's/fuse3/fused/g' meson_options.txt

Choose a reason for hiding this comment

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

Ditto on doing this in the fork.


%prep
%autosetup
find . -type f -name "*" -exec sed -i 's/fuse3/fused/g' {} ';'

Choose a reason for hiding this comment

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

Ditto on using xargs and doing it in the fork.

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