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

core/common: Fix ENODATA for FreeBSD with compat.h #15685

Merged
merged 1 commit into from Jun 19, 2017

Conversation

Projects
None yet
4 participants
@wjwithagen
Contributor

wjwithagen commented Jun 14, 2017

  • When ENODATA used, compat.h needs to be include before any
    of the includes that could possibly define ENODATA by itself.
    Possible problems stem from including Boost files
    Since otherwise xattr-tests will fail to detect that attributes
    are (not) there.

Tracker: http://tracker.ceph.com/issues/19883
Signed-off-by: Willem Jan Withagen wjw@digiware.nl

@@ -1,6 +1,7 @@
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
// vim: ts=8 sw=2 smarttab
#include "compat.h"

This comment has been minimized.

@tchaikov

tchaikov Jun 15, 2017

Contributor

might want to put include/compact to more consistent?

This comment has been minimized.

@wjwithagen

wjwithagen Jun 15, 2017

Contributor

@tchaikov
mmm, thougth I caught all these slip-ups. Will fix.

@@ -4,6 +4,7 @@
#ifndef __CEPH_OSD_CHAIN_XATTR_H
#define __CEPH_OSD_CHAIN_XATTR_H
#include "include/compat.h"

This comment has been minimized.

@tchaikov

tchaikov Jun 15, 2017

Contributor

any specific reason you want to put #include "include/compat.h" before other includes? it seems we have a tradition to include system headers first, other 3rd party headers, and then headers in ceph project.

okay, http://tracker.ceph.com/issues/19883#note-6 explains the rationale.

This comment has been minimized.

@wjwithagen

wjwithagen Jun 15, 2017

Contributor

@tchaikov
I will put the rationale in both PR and commit...

core/common: Fix ENODATA for FreeBSD with compat.h
 - When ENODATA used, compat.h needs to be include before any
   of the includes that could possibly define ENODATA by itself.
   Since otherwise xattr-tests will fail to detect that attributes
   are (not) there.

Tracker: http://tracker.ceph.com/issues/19883
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@joscollin

The changes looks good considering the rationale.

Is the PR Title Prefix and the Commit Title Prefix correct (core/common) ? Because the changes are in os, rgw, test and tools submodules. Please verify. Please correct me if I misunderstood something.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 16, 2017

@joscollin
That is why I speced it as core/common. Otherwise you get these huge lists of parts.
Done that for most of these invasive PRs.

@liewegas liewegas merged commit 5c04125 into ceph:master Jun 19, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
arm64 make check arm64 make check succeeded
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment