Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1955)

Issue 7305072: code review 7305072: archive/tar: add Header.FileInfo method. Add more cases... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 9 months ago by u
Modified:
8 years, 9 months ago
Reviewers:
CC:
golang-dev, donovanhide_gmail.com, minux1, adg
Visibility:
Public.

Description

archive/tar: add Header.FileInfo method. Add more cases to FileInfoHeader. FileInfoHeader can now handle fifo, setuid, setgid and sticky bits. Fixes issue 4695.

Patch Set 1 #

Patch Set 2 : diff -r 85d759cdb33d https://code.google.com/p/go #

Total comments: 5

Patch Set 3 : diff -r 9f7179fbede0 https://code.google.com/p/go #

Total comments: 3

Patch Set 4 : diff -r a44171137af8 https://code.google.com/p/go #

Patch Set 5 : diff -r 83bea0ad80a2 https://code.google.com/p/go #

Patch Set 6 : diff -r 83bea0ad80a2 https://code.google.com/p/go #

Patch Set 7 : diff -r d89c5a625fb5 https://code.google.com/p/go #

Patch Set 8 : diff -r 21716d4d42ab https://code.google.com/p/go #

Patch Set 9 : diff -r d4024b084e0c https://code.google.com/p/go #

Patch Set 10 : diff -r d4024b084e0c https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+271 lines, -14 lines) Patch
M src/pkg/archive/tar/common.go View 1 2 3 4 5 6 7 8 4 chunks +124 lines, -14 lines 0 comments Download
M src/pkg/archive/tar/tar_test.go View 1 2 3 4 5 6 7 8 1 chunk +147 lines, -0 lines 0 comments Download

Messages

Total messages: 18
u
https://codereview.appspot.com/7305072/diff/2001/src/pkg/archive/tar/common.go File src/pkg/archive/tar/common.go (right): https://codereview.appspot.com/7305072/diff/2001/src/pkg/archive/tar/common.go#newcode83 src/pkg/archive/tar/common.go:83: mode |= os.ModeNamedPipe Could someone please verify that os.ModeNamedPipe ...
8 years, 9 months ago (2013-02-09 20:21:11 UTC) #1
u
Hello golang-dev@googlegroups.com (cc: adg@golang.org), I'd like you to review this change to https://code.google.com/p/go
8 years, 9 months ago (2013-02-09 20:22:44 UTC) #2
Donovan
https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go File src/pkg/archive/tar/common.go (right): https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go#newcode75 src/pkg/archive/tar/common.go:75: mode = os.FileMode(fi.h.Mode) & os.ModePerm It's a bit obscure, ...
8 years, 9 months ago (2013-02-10 11:14:48 UTC) #3
minux1
tests? https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go File src/pkg/archive/tar/common.go (right): https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go#newcode75 src/pkg/archive/tar/common.go:75: mode = os.FileMode(fi.h.Mode) & os.ModePerm On 2013/02/10 11:14:48, ...
8 years, 9 months ago (2013-02-10 18:22:17 UTC) #4
u
Hello golang-dev@googlegroups.com, donovanhide@gmail.com, r.eklind.87@gmail.com, minux.ma@gmail.com (cc: adg@golang.org, golang-dev@googlegroups.com), Please take another look.
8 years, 9 months ago (2013-02-10 18:43:07 UTC) #5
u
On 2013/02/10 18:22:17, minux wrote: > tests? I want to include test but don't know ...
8 years, 9 months ago (2013-02-10 18:45:00 UTC) #6
u
On 2013/02/10 11:14:48, Donovan wrote: > https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go > File src/pkg/archive/tar/common.go (right): > > https://codereview.appspot.com/7305072/diff/7001/src/pkg/archive/tar/common.go#newcode75 > ...
8 years, 9 months ago (2013-02-10 18:47:17 UTC) #7
adg
On 2013/02/10 18:45:00, u wrote: > On 2013/02/10 18:22:17, minux wrote: > > tests? > ...
8 years, 9 months ago (2013-02-11 01:10:53 UTC) #8
u
Hello golang-dev@googlegroups.com, donovanhide@gmail.com, r.eklind.87@gmail.com, minux.ma@gmail.com, adg@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
8 years, 9 months ago (2013-02-11 11:12:47 UTC) #9
adg
Looks good, but what about the roundtrip test that I suggested?
8 years, 9 months ago (2013-02-12 03:43:15 UTC) #10
u
Hello golang-dev@googlegroups.com, donovanhide@gmail.com, r.eklind.87@gmail.com, minux.ma@gmail.com, adg@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
8 years, 9 months ago (2013-02-13 06:12:35 UTC) #11
u
TestHeaderRoundTrip has now been implemented. FileInfoHeader has also been extended to fifo, setuid, setgid and ...
8 years, 9 months ago (2013-02-13 06:13:36 UTC) #12
u
Hello golang-dev@googlegroups.com, donovanhide@gmail.com, r.eklind.87@gmail.com, minux.ma@gmail.com, adg@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
8 years, 9 months ago (2013-02-13 11:48:11 UTC) #13
u
Updated to reflect changes in rev d4024b084e0c.
8 years, 9 months ago (2013-02-13 11:48:53 UTC) #14
adg
LGTM Great work, thanks.
8 years, 9 months ago (2013-02-13 22:36:54 UTC) #15
u
On 2013/02/13 22:36:54, adg wrote: > LGTM > > Great work, thanks. Thank you. The ...
8 years, 9 months ago (2013-02-14 05:30:16 UTC) #16
adg
*** Submitted as https://code.google.com/p/go/source/detail?r=de92672228d3 *** archive/tar: add Header.FileInfo method. Add more cases to FileInfoHeader. FileInfoHeader ...
8 years, 9 months ago (2013-02-14 06:36:10 UTC) #17
u
8 years, 9 months ago (2013-02-22 08:14:52 UTC) #18
Message was sent while issue was closed.
*** Abandoned ***
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b