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

fix(build): check features before mangling CFLAGS #589

Merged
1 commit merged into from
Sep 27, 2024

Conversation

ghost
Copy link

@ghost ghost commented Sep 11, 2024

Stacked PRs:


fix(build): check features before mangling CFLAGS

Yet another autotools fix: If any warning/devel CFLAGS would invoke
warnings in headers used to detect dependencies, those dependencies will
fail with a highly misleading/confusing error message:

configure: Found .git directory. Adding -Werror to CFLAGS.
checking if running on EC2 instance... yes
checking if want AWS platform optimizations... yes
checking for Libfabric 1.18.0 or later... no
configure: error: On AWS platforms, Libfabric 1.18.0 or later is required

ie: it is not that Libfabric 1.18 was not found, it was that its headers
produced warnings. Fix this by resolving all dependencies before
modifying CFLAGS.

@ghost ghost requested a review from bwbarrett as a code owner September 11, 2024 23:12
@ghost ghost force-pushed the aws-nslick/stack/31 branch from 5beaf9d to b22616e Compare September 11, 2024 23:12
@ghost ghost changed the base branch from aws-nslick/stack/29 to master September 12, 2024 00:17
@ghost ghost requested review from rajachan, rauteric and a team as code owners September 12, 2024 00:17
@ghost ghost force-pushed the aws-nslick/stack/31 branch from b22616e to 7c86788 Compare September 12, 2024 00:17
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 12, 2024 00:18
@ghost ghost mentioned this pull request Sep 12, 2024
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 12, 2024 01:10
@ghost ghost changed the base branch from aws-nslick/stack/29 to master September 12, 2024 01:51
@ghost ghost force-pushed the aws-nslick/stack/31 branch from 22cf81b to 78cc90c Compare September 12, 2024 01:51
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 12, 2024 01:52
@ghost ghost added the previously-passed-ci label Sep 13, 2024
@ghost ghost changed the base branch from aws-nslick/stack/29 to master September 13, 2024 17:49
@ghost ghost force-pushed the aws-nslick/stack/31 branch from 78cc90c to 189af46 Compare September 13, 2024 17:49
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 13, 2024 17:50
rauteric
rauteric previously approved these changes Sep 13, 2024
@ghost ghost changed the base branch from aws-nslick/stack/29 to master September 14, 2024 22:36
@ghost ghost dismissed rauteric’s stale review September 14, 2024 22:36

The base branch was changed.

@ghost ghost force-pushed the aws-nslick/stack/31 branch from 189af46 to a4650f5 Compare September 14, 2024 22:37
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 14, 2024 22:38
@ghost ghost changed the base branch from aws-nslick/stack/29 to master September 17, 2024 20:25
@ghost ghost force-pushed the aws-nslick/stack/31 branch from a4650f5 to 201ec50 Compare September 17, 2024 20:25
@ghost ghost changed the base branch from master to aws-nslick/stack/29 September 17, 2024 20:26
configure.ac Outdated
NCCL_OFI_PLATFORM="none"
AS_IF([test "${NCCL_OFI_PLATFORM}" = "none"], [AX_CHECK_PLATFORM_AWS()])

AC_SUBST([NCCL_NET_OFI_DISTCHCK_CONFIGURE_FLAGS])
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not move up. The rest makes sense to me.

Yet another autotools fix: If any warning/devel CFLAGS would invoke
warnings in headers used to detect dependencies, those dependencies will
fail with a highly misleading/confusing error message:

> configure: Found .git directory.  Adding -Werror to CFLAGS.
> checking if running on EC2 instance... yes
> checking if want AWS platform optimizations... yes
> checking for Libfabric 1.18.0 or later... no
> configure: error: On AWS platforms, Libfabric 1.18.0 or later is required

ie: it is not that Libfabric 1.18 was not found, it was that its headers
produced warnings. Fix this by resolving all dependencies before
modifying CFLAGS.

Signed-off-by: Nicholas Sielicki <nslick@amazon.com>
@ghost ghost mentioned this pull request Sep 27, 2024
@ghost ghost force-pushed the aws-nslick/stack/29 branch from 0dace6a to e9b6309 Compare September 27, 2024 00:33
@ghost ghost force-pushed the aws-nslick/stack/31 branch from 5504efb to 839fa16 Compare September 27, 2024 00:33
@ghost ghost requested a review from bwbarrett September 27, 2024 01:28
@ghost ghost dismissed bwbarrett’s stale review September 27, 2024 01:44

outdated

@ghost
Copy link
Author

ghost commented Sep 27, 2024

bot:aws:retest

@ghost ghost merged commit 278bfdb into aws-nslick/stack/29 Sep 27, 2024
30 of 31 checks passed
@ghost ghost deleted the aws-nslick/stack/31 branch September 27, 2024 22:38
ghost pushed a commit that referenced this pull request Oct 1, 2024
Yet another autotools fix: If any warning/devel CFLAGS would invoke
warnings in headers used to detect dependencies, those dependencies will
fail with a highly misleading/confusing error message:

> configure: Found .git directory.  Adding -Werror to CFLAGS.
> checking if running on EC2 instance... yes
> checking if want AWS platform optimizations... yes
> checking for Libfabric 1.18.0 or later... no
> configure: error: On AWS platforms, Libfabric 1.18.0 or later is required

ie: it is not that Libfabric 1.18 was not found, it was that its headers
produced warnings. Fix this by resolving all dependencies before
modifying CFLAGS.

Signed-off-by: Nicholas Sielicki <nslick@amazon.com>
aws-ofiwg-bot pushed a commit to aws-ofiwg-bot/aws-ofi-nccl that referenced this pull request Oct 4, 2024
Yet another autotools fix: If any warning/devel CFLAGS would invoke
warnings in headers used to detect dependencies, those dependencies will
fail with a highly misleading/confusing error message:

> configure: Found .git directory.  Adding -Werror to CFLAGS.
> checking if running on EC2 instance... yes
> checking if want AWS platform optimizations... yes
> checking for Libfabric 1.18.0 or later... no
> configure: error: On AWS platforms, Libfabric 1.18.0 or later is required

ie: it is not that Libfabric 1.18 was not found, it was that its headers
produced warnings. Fix this by resolving all dependencies before
modifying CFLAGS.

Signed-off-by: Nicholas Sielicki <nslick@amazon.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants