add libfabric #5914

dvirtz · 2021-06-15T20:49:04Z

Specify library name and version: libfabric/1.12.1 and libfabric/1.9.0amzn1.1

This is a dependency of AWS CDI SDK which I plan to add next.

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

intelligide · 2021-06-16T04:19:00Z

recipes/libfabric/all/conandata.yml

+  "1.9.0amzn1.1":
+    url: https://github.com/aws/libfabric/releases/download/v1.9.0amzn1.1/libfabric-1.9.0amzn1.1.tar.gz
+    sha256: 4e3912c379ca351f5039b01cfd168750030d919f1ce3f46531b5396b6a4266e5


For forks, create a separate recipe named aws-libfabric

does it require a separate PR?

uilianries

@dvirtz Thank you for your contribution! Please, take a look on my review

recipes/libfabric/all/conanfile.py

uilianries · 2021-06-16T18:27:50Z

recipes/libfabric/all/conanfile.py

+        if self.settings.compiler == "Visual Studio":
+            raise ConanInvalidConfiguration("The libfabric package cannot be built on Visual Studio.")
+        for p in self._providers:
+            if self.options.get_safe(p) not in ["auto", "yes", "no", "dl"] and not os.path.isdir(str(self.options.get_safe(p))):


Suggested change

if self.options.get_safe(p) not in ["auto", "yes", "no", "dl"] and not os.path.isdir(str(self.options.get_safe(p))):

if self.options.get_safe(p) not in ["auto", "yes", "no", "dl"] and not os.path.isdir(str(self.options.get_safe(p))):

I would prefer listing only what's allowed instead of ANY, it seems clear what is valid here, for instance ["auto", True, False, "dl"]

uilianries · 2021-06-16T18:34:51Z

recipes/libfabric/all/conanfile.py

+    settings = "os", "arch", "compiler", "build_type"
+    _providers = ['gni', 'psm', 'psm2', 'psm3', 'rxm', 'sockets', 'tcp', 'udp', 'usnic', 'verbs', 'bgq']
+    options = {
+        **{ p: "ANY" for p in _providers },


Suggested change

**{ p: "ANY" for p in _providers },

**{ p: [True, False, "auto", "dl"] for p in _providers },

read comment below.
Also, if auto has same effect as True or False, it should have the same package id! For example:

conan-center-index/recipes/getdns/all/conanfile.py

Line 48 in 41d4fbb

def _stub_only(self):

I used ANY because it can also be a directory path

I can't know if auto will resolve to True of False before running configuration

recipes/libfabric/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

jgsogo · 2021-06-18T17:36:19Z

Hi, probably the recipe is almost ready, but CLA is not reporting status... I will close and open the PR to see if it gets updated.

prince-chrismc

One small comment

prince-chrismc · 2021-06-22T23:21:06Z

recipes/libfabric/all/conanfile.py

+            raise ConanInvalidConfiguration("Value of with_libnl must be an existing directory")
+
+    def source(self):
+        tools.get(**self.conan_data["sources"][self.version])


Please replace these 3 lines with our strip_root utility

dvirtz · 2021-06-23T17:20:58Z

build failed but no logs

prince-chrismc · 2021-06-23T21:18:11Z

You'll need to retrigger CI, close the pr wait 10s and then re-open it 🔁

conan-center-bot · 2021-06-28T19:03:32Z

All green in build 11 (9344d5a8478b895b7bdcd35456531f99bd65433b):

libfabric/1.12.1@:
All packages built successfully! (All logs)

add libfabric

6dba3b5

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jun 15, 2021

del fPIC when shared

c6bdf31

This comment has been minimized.

Sign in to view

intelligide suggested changes Jun 16, 2021

View reviewed changes

dvirtz added 2 commits June 16, 2021 12:03

remove aws fork

cea55f5

fix hook errors

1681958

This comment has been minimized.

Sign in to view

don't require C++11

064a7d5

This comment has been minimized.

Sign in to view

conan-center-bot removed the Failed label Jun 16, 2021

SSE4 requested a review from uilianries June 16, 2021 14:56

uilianries reviewed Jun 16, 2021

View reviewed changes

SpaceIm reviewed Jun 16, 2021

View reviewed changes

recipes/libfabric/all/conanfile.py Show resolved Hide resolved

dvirtz and others added 2 commits June 17, 2021 08:37

Apply suggestions from code review

b8e902f

Co-authored-by: Uilian Ries <uilianries@gmail.com>

Update recipes/libfabric/all/conanfile.py

ab0f7b3

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

This comment has been minimized.

Sign in to view

jgsogo closed this Jun 18, 2021

jgsogo reopened this Jun 18, 2021

This comment has been minimized.

Sign in to view

dvirtz mentioned this pull request Jun 22, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

prince-chrismc reviewed Jun 22, 2021

View reviewed changes

use strip_root

9344d5a

prince-chrismc approved these changes Jun 23, 2021

View reviewed changes

dvirtz closed this Jun 23, 2021

dvirtz reopened this Jun 23, 2021

This comment has been minimized.

Sign in to view

conan-center-bot requested review from SSE4 and jgsogo June 24, 2021 11:59

dvirtz closed this Jun 28, 2021

dvirtz reopened this Jun 28, 2021

jgsogo approved these changes Jun 29, 2021

View reviewed changes

conan-center-bot requested a review from memsharded June 29, 2021 11:59

uilianries approved these changes Jun 29, 2021

View reviewed changes

conan-center-bot merged commit 02d64a7 into conan-io:master Jun 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add libfabric #5914

add libfabric #5914

dvirtz commented Jun 15, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

intelligide Jun 16, 2021

dvirtz Jun 16, 2021

This comment has been minimized.

This comment has been minimized.

uilianries left a comment

uilianries Jun 16, 2021

uilianries Jun 16, 2021

dvirtz Jun 17, 2021 •

edited

Loading

dvirtz Jun 17, 2021

This comment has been minimized.

jgsogo commented Jun 18, 2021

This comment has been minimized.

prince-chrismc left a comment

prince-chrismc Jun 22, 2021

dvirtz commented Jun 23, 2021

prince-chrismc commented Jun 23, 2021

This comment has been minimized.

conan-center-bot commented Jun 28, 2021

	if self.options.get_safe(p) not in ["auto", "yes", "no", "dl"] and not os.path.isdir(str(self.options.get_safe(p))):
	if self.options.get_safe(p) not in ["auto", "yes", "no", "dl"] and not os.path.isdir(str(self.options.get_safe(p))):

	**{ p: "ANY" for p in _providers },
	**{ p: [True, False, "auto", "dl"] for p in _providers },

add libfabric #5914

add libfabric #5914

Conversation

dvirtz commented Jun 15, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

intelligide Jun 16, 2021

Choose a reason for hiding this comment

dvirtz Jun 16, 2021

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

uilianries left a comment

Choose a reason for hiding this comment

uilianries Jun 16, 2021

Choose a reason for hiding this comment

uilianries Jun 16, 2021

Choose a reason for hiding this comment

dvirtz Jun 17, 2021 • edited Loading

Choose a reason for hiding this comment

dvirtz Jun 17, 2021

Choose a reason for hiding this comment

This comment has been minimized.

jgsogo commented Jun 18, 2021

This comment has been minimized.

prince-chrismc left a comment

Choose a reason for hiding this comment

prince-chrismc Jun 22, 2021

Choose a reason for hiding this comment

dvirtz commented Jun 23, 2021

prince-chrismc commented Jun 23, 2021

This comment has been minimized.

conan-center-bot commented Jun 28, 2021

dvirtz commented Jun 15, 2021 •

edited

Loading

dvirtz Jun 17, 2021 •

edited

Loading