Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-05-01 Thread Andrew Schwartzmeyer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/
---

(Updated May 1, 2018, 2:23 p.m.)


Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.


Changes
---

Increased verbosity level of diagnostic message.


Bugs: MESOS-8275
https://issues.apache.org/jira/browse/MESOS-8275


Repository: mesos


Description
---

The functions `mode()`, `dev()`, and `inode()` are unused and do not
make sense on Windows, so they were explicitly deleted. The function
`mtime()` is used and has a logical mapping, `GetFileTime()`. However,
Windows reports time differently from POSIX, so a conversion must also
be performed such that the API `os::stat::mtime()` remains consistent
with its POSIX version.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/permissions.hpp 
453e60c7268db516c2c94501e11a92fe8f490498 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
  3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 


Diff: https://reviews.apache.org/r/66773/diff/4/

Changes: https://reviews.apache.org/r/66773/diff/3-4/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-05-01 Thread Andrew Schwartzmeyer 


> On May 1, 2018, 12:16 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/permissions.hpp
> > Lines 64 (patched)
> > 
> >
> > I have a feeling this will pollute the logs unless you demote it to 
> > `VLOG(2)` or similar.  It doesn't seem too useful of a warning at runtime.

Yeahhh fair enough.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review202206
---


On April 30, 2018, 1:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 30, 2018, 1:52 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-05-01 Thread Joseph Wu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review202206
---


Fix it, then Ship it!





3rdparty/stout/include/stout/os/permissions.hpp
Lines 64 (patched)


I have a feeling this will pollute the logs unless you demote it to 
`VLOG(2)` or similar.  It doesn't seem too useful of a warning at runtime.


- Joseph Wu


On April 30, 2018, 1:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 30, 2018, 1:52 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-30 Thread Andrew Schwartzmeyer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/
---

(Updated April 30, 2018, 1:52 p.m.)


Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.


Changes
---

Fixed math in comment.


Bugs: MESOS-8275
https://issues.apache.org/jira/browse/MESOS-8275


Repository: mesos


Description
---

The functions `mode()`, `dev()`, and `inode()` are unused and do not
make sense on Windows, so they were explicitly deleted. The function
`mtime()` is used and has a logical mapping, `GetFileTime()`. However,
Windows reports time differently from POSIX, so a conversion must also
be performed such that the API `os::stat::mtime()` remains consistent
with its POSIX version.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/permissions.hpp 
453e60c7268db516c2c94501e11a92fe8f490498 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
  3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 


Diff: https://reviews.apache.org/r/66773/diff/3/

Changes: https://reviews.apache.org/r/66773/diff/2-3/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-30 Thread Andrew Schwartzmeyer 


> On April 30, 2018, 11:38 a.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/os/windows/stat.hpp
> > Lines 186 (patched)
> > 
> >
> > the comment should be `x / (10 * 1000 * 1000)` :)
> 
> Andrew Schwartzmeyer wrote:
> Oops.

I dropped a 1000. I meant `(x * 100) / (1000^3)`, as in `x 100-nanosecond 
intervals to nanoseconds is x * 100` and then `x nanoseconds / 1000^3` to 
seconds.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review202132
---


On April 26, 2018, 9:21 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 26, 2018, 9:21 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-30 Thread Akash Gupta 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review202132
---


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/stat.hpp
Lines 186 (patched)


the comment should be `x / (10 * 1000 * 1000)` :)


- Akash Gupta


On April 27, 2018, 4:21 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 27, 2018, 4:21 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-27 Thread John Kordich via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review202085
---


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:21 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 27, 2018, 4:21 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-26 Thread Andrew Schwartzmeyer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/
---

(Updated April 26, 2018, 9:21 p.m.)


Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.


Changes
---

Rebased.


Bugs: MESOS-8275
https://issues.apache.org/jira/browse/MESOS-8275


Repository: mesos


Description
---

The functions `mode()`, `dev()`, and `inode()` are unused and do not
make sense on Windows, so they were explicitly deleted. The function
`mtime()` is used and has a logical mapping, `GetFileTime()`. However,
Windows reports time differently from POSIX, so a conversion must also
be performed such that the API `os::stat::mtime()` remains consistent
with its POSIX version.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/permissions.hpp 
453e60c7268db516c2c94501e11a92fe8f490498 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
  3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 


Diff: https://reviews.apache.org/r/66773/diff/2/

Changes: https://reviews.apache.org/r/66773/diff/1-2/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-24 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review201852
---



Patch looks great!

Reviews applied: [66420, 66421, 66422, 66423, 66424, 66425, 66426, 66427, 
66428, 66455, 66429, 66430, 66431, 66432, 66434, 66435, 66436, 66437, 66433, 
66438, 66439, 66440, 66442, 66443, 66444, 66445, 66578, 66641, 66773]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 24, 2018, 5:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 24, 2018, 5:12 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-24 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/#review201850
---



PASS: Mesos patch 66773 was successfully built and tested.

Reviews applied: `['66423', '66424', '66425', '66426', '66427', '66428', 
'66455', '66429', '66430', '66431', '66432', '66434', '66435', '66436', 
'66437', '66433', '66438', '66439', '66440', '66442', '66443', '66444', 
'66445', '66578', '66641', '66773']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66773

- Mesos Reviewbot Windows


On April 24, 2018, 10:12 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 24, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-24 Thread Andrew Schwartzmeyer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66773/
---

Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.


Bugs: MESOS-8275
https://issues.apache.org/jira/browse/MESOS-8275


Repository: mesos


Description
---

The functions `mode()`, `dev()`, and `inode()` are unused and do not
make sense on Windows, so they were explicitly deleted. The function
`mtime()` is used and has a logical mapping, `GetFileTime()`. However,
Windows reports time differently from POSIX, so a conversion must also
be performed such that the API `os::stat::mtime()` remains consistent
with its POSIX version.


Diffs
-

  3rdparty/stout/include/stout/os/permissions.hpp 
453e60c7268db516c2c94501e11a92fe8f490498 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
  3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 


Diff: https://reviews.apache.org/r/66773/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer