Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

tests: fix xattr.h include directive #253

Merged
merged 1 commit into from Jul 17, 2017

Conversation

GBuella
Copy link
Contributor

@GBuella GBuella commented Jul 17, 2017

Ref: #252


This change is Reviewable

@codecov-io
Copy link

codecov-io commented Jul 17, 2017

Codecov Report

Merging #253 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
- Coverage   78.61%   78.59%   -0.03%     
==========================================
  Files          75       75              
  Lines       11388    11391       +3     
  Branches     1570     1570              
==========================================
  Hits         8953     8953              
- Misses       1842     1843       +1     
- Partials      593      595       +2
Impacted Files Coverage Δ
tests/preload/xattr/xattr.c 32% <ø> (ø) ⬆️
src/libpmemfile/syscall_early_filter.c 60% <0%> (-20%) ⬇️
src/libpmemfile-posix/inode.c 90.17% <0%> (-0.43%) ⬇️
src/libpmemfile-posix/file.c 65.92% <0%> (-0.28%) ⬇️
src/libpmemfile/preload.c 42.57% <0%> (-0.19%) ⬇️
src/libpmemfile-posix/data.c 91.96% <0%> (-0.04%) ⬇️
src/libpmemfile-posix/read.c 94.3% <0%> (+0.03%) ⬆️
src/libpmemfile-posix/dir.c 82.26% <0%> (+0.24%) ⬆️
src/libpmemfile-posix/utils.c 72.85% <0%> (+0.39%) ⬆️
src/libpmemfile-posix/rename.c 91.16% <0%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d3ef0a...116bc41. Read the comment docs.

@marcinslusarz
Copy link
Member

Please also revert libattr1-dev installation from Docker images.

@krzycz
Copy link
Contributor

krzycz commented Jul 17, 2017

:lgtm:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@marcinslusarz
Copy link
Member

marcinslusarz commented Jul 17, 2017

tests/preload/xattr/xattr.c, line 48 at r2 (raw file):

#include <sys/types.h>
#include <unistd.h>
#include <sys/xattr.h>

If this file is not buggy anymore you should a) put it in a proper place in the include list and b) trim the list of header files. If it still is, then don't remove the comment.


Comments from Reviewable

Also removing libattr-dev from the list of packages to
be installed for docker builds.
The confusion was due debian/ubuntu not providing a
man page for setxattr/getxattr by default, and a man
page installed with libattr-dev. This man page suggests
the attr/xattr.h header.
@GBuella
Copy link
Contributor Author

GBuella commented Jul 17, 2017

Review status: 0 of 8 files reviewed at latest revision, 1 unresolved discussion.


tests/preload/xattr/xattr.c, line 48 at r2 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

If this file is not buggy anymore you should a) put it in a proper place in the include list and trim the list of header files. If it still is, then don't remove the comment.

I managed to get rid of stddef.h. The others are needed for PATH_MAX, open, sprintf, etc...


Comments from Reviewable

@marcinslusarz
Copy link
Member

:lgtm:


Reviewed 7 of 7 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@marcinslusarz
Copy link
Member

@sarahjelinek Please take a look at this PR ASAP. It's affecting all Jenkins jobs.

@sarahjelinek sarahjelinek merged commit f8f7296 into pmem:master Jul 17, 2017
@ldorau
Copy link
Member

ldorau commented Jul 18, 2017

🥇 👍

@ldorau
Copy link
Member

ldorau commented Jul 18, 2017

:lgtm:


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants