Skip to content

[release/1.1 backport] fix: SCHILY.xattrs should be SCHILY.xattr#2954

Merged
estesp merged 1 commit intocontainerd:release/1.1from
thaJeztah:1.1_backport_fix_xattr
Jan 24, 2019
Merged

[release/1.1 backport] fix: SCHILY.xattrs should be SCHILY.xattr#2954
estesp merged 1 commit intocontainerd:release/1.1from
thaJeztah:1.1_backport_fix_xattr

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Jan 23, 2019 •
edited
Loading

backport of #2873 for 1.1. Relates to #2942

cherry-pick was not clean conflicts in archive/tar_test.go, due to b6107dc (#2696) missing, which moved some things to archive/tartest. I resolved those conflicts by removing the tartest. package name from the test, and removed the import.

from golang code
https://github.com/golang/go/blob/bad6b6fa9190e9079a6d6958859856a66f0fab87/src/archive/tar/common.go#L110

add unit test for tar xattr

Fixes: #2863

(linking for discoverability); Introduced in bc9cb25 (#1812)

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jan 23, 2019

Let's see if it goes green @estesp 👍

@thaJeztah thaJeztah mentioned this pull request Jan 23, 2019
Merged
@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jan 24, 2019

Still shows a missing dependencies

@thaJeztah thaJeztah force-pushed the 1.1_backport_fix_xattr branch from 3a1a884 to b48afb4 Compare January 24, 2019 01:29
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jan 24, 2019 

archive/tar_test.go:36:2:warning: could not import github.com/containerd/containerd/pkg/testutil (invalid package name: "") (compile) (staticcheck)
archive/tar_test.go:36:2:warning: could not import github.com/containerd/containerd/pkg/testutil (invalid package name: "") (compile) (staticcheck)
archive/tar_test.go:36:2:warning: unused variable or constant could not import github.com/containerd/containerd/pkg/testutil (cannot find package "github.com/containerd/containerd/pkg/testutil" in any of: (varcheck)
make: *** [check] Error 1
The command "make check" exited with 2.

Ah! It was moved from /testutil to /pkg/testutil in #2351

updated

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 24, 2019

Codecov Report

Merging #2954 into release/1.1 will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/1.1    #2954      +/-   ##
===============================================
+ Coverage        48.98%   49.05%   +0.07%     
===============================================
  Files               85       85              
  Lines             7601     7601              
===============================================
+ Hits              3723     3729       +6     
+ Misses            3203     3197       -6     
  Partials           675      675
Flag Coverage Δ
#linux 49.05% <ø> (+0.07%) ⬆️
Impacted Files Coverage Δ
archive/tar.go 50.89% <ø> (+0.89%) ⬆️
archive/tar_unix.go 59.09% <0%> (+4.54%) ⬆️

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 9979a1a...b48afb4. Read the comment docs.

fuweid
fuweid reviewed Jan 24, 2019
View reviewed changes
Comment thread archive/tar_test.go
@@ -32,6 +33,7 @@ import (

Copy link
Copy Markdown
Member

@fuweid fuweid Jan 24, 2019

Choose a reason for hiding this comment

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

could we remove the empty here? since we don't need to separate the packages into 3 groups 😄

Copy link
Copy Markdown
Member Author

@thaJeztah thaJeztah Jan 24, 2019

Choose a reason for hiding this comment

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

Best done on master; this is only a backport of the fix, and I'd rather not make those changes in the release branch; master also has it split into 3 groups (which was done in the original PR adding these); 0deba01

Copy link
Copy Markdown
Member

@fuweid fuweid Jan 24, 2019

Choose a reason for hiding this comment

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

make senses! thanks.

fuweid
fuweid reviewed Jan 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

estesp
estesp approved these changes Jan 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 813e5f6 into containerd:release/1.1 Jan 24, 2019
@thaJeztah thaJeztah deleted the 1.1_backport_fix_xattr branch January 24, 2019 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fuweid fuweid fuweid left review comments

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@thaJeztah @dmcgowan @codecov-io @estesp @fuweid @Ace-Tang