base::GetTempDir does not respect $TMPDIR on OS X |
||||
Issue descriptionAs part of the continuous integration for chromium, we make a best-effort attempt to sandbox tests, especially on swarming. One of the ways that we do this is by setting $TMPDIR to freshly-allocated directories that we can clean up after the task completes (https://github.com/luci/luci-py/issues/157). I was poking around at this code recently, and discovered that base doesn't inspect $TMPDIR at all on OS X. Windows respects $TEMP (via GetTempPath() https://msdn.microsoft.com/en-us/library/windows/desktop/aa364992(v=vs.85).aspx), and Linux respects $TMPDIR (via direct inspection of $TMPDIR https://cs.chromium.org/chromium/src/base/files/file_util_posix.cc?l=530). However on OS X, base::GetTempDir (https://cs.chromium.org/chromium/src/base/files/file_util_mac.mm?q=file_util_mac+package:%5Echromium$&l=25) is implemented by calling NSTemporaryDirectory (https://developer.apple.com/reference/foundation/1409211-nstemporarydirectory). This function is implemented by internally inspecting the confstr(_CS_DARWIN_USER_TEMP_DIR) value, which has no discernible way to override it. In this CL (https://codereview.chromium.org/2713293002/) I propose to simply check $TMPDIR first, falling back to NSTemporaryDirectory if the envvar is unset. This would mimic the behavior we currently use for Linux (which checks $TMPDIR and then falls back to either a hard-coded "/tmp", or `PathService::Get(base::DIR_CACHE, path)` on android).
,
Feb 28 2017
Nice, the proposal sounds good.
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a commit 7b5db573b3ebbc32ee4608163b9ba7e68750ee9a Author: iannucci <iannucci@chromium.org> Date: Wed Mar 01 03:53:19 2017 Allow $TMPDIR to set temporary directory on OS X. This allows the continuous integration builds to be more hermetic by supplying a managed value for $TMPDIR and having chromium + tests respect it. NSTemporaryDirectory takes its value directly from a non-overridable confstr(_CS_DARWIN_USER_TEMP_DIR) call internally. This CL brings OS X more in line with Windows and Linux which both respect an environment variable ($TEMP and $TMPDIR respectively). BUG= 696803 Review-Url: https://codereview.chromium.org/2713293002 Cr-Commit-Position: refs/heads/master@{#453835} [modify] https://crrev.com/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a/base/files/file_util_mac.mm [modify] https://crrev.com/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a/chrome/browser/file_select_helper_mac.mm [modify] https://crrev.com/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a/remoting/host/mac/me2me_preference_pane.mm
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cedd448c905837fc74d0b793305733da3c88e900 commit cedd448c905837fc74d0b793305733da3c88e900 Author: iannucci <iannucci@chromium.org> Date: Wed Mar 01 04:44:59 2017 Revert of Allow $TMPDIR to set temporary directory on OS X. (patchset #8 id:140001 of https://codereview.chromium.org/2713293002/ ) Reason for revert: Looks like this broke mac compile https://build.chromium.org/p/chromium/buildstatus?builder=Mac&number=24213 Original issue's description: > Allow $TMPDIR to set temporary directory on OS X. > > This allows the continuous integration builds to be more hermetic by > supplying a managed value for $TMPDIR and having chromium + tests respect > it. > > NSTemporaryDirectory takes its value directly from a non-overridable > confstr(_CS_DARWIN_USER_TEMP_DIR) call internally. This CL brings OS X > more in line with Windows and Linux which both respect an environment > variable ($TEMP and $TMPDIR respectively). > > BUG= 696803 > > Review-Url: https://codereview.chromium.org/2713293002 > Cr-Commit-Position: refs/heads/master@{#453835} > Committed: https://chromium.googlesource.com/chromium/src/+/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a TBR=lambroslambrou@chromium.org,jam@chromium.org,maruel@chromium.org,mark@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 696803 Review-Url: https://codereview.chromium.org/2724733002 Cr-Commit-Position: refs/heads/master@{#453841} [modify] https://crrev.com/cedd448c905837fc74d0b793305733da3c88e900/base/files/file_util_mac.mm [modify] https://crrev.com/cedd448c905837fc74d0b793305733da3c88e900/chrome/browser/file_select_helper_mac.mm [modify] https://crrev.com/cedd448c905837fc74d0b793305733da3c88e900/remoting/host/mac/me2me_preference_pane.mm
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95fcf47ca01054ffe38e8609525a6ca8579b7824 commit 95fcf47ca01054ffe38e8609525a6ca8579b7824 Author: iannucci <iannucci@chromium.org> Date: Wed Mar 01 07:00:32 2017 Allow $TMPDIR to set temporary directory on OS X. This allows the continuous integration builds to be more hermetic by supplying a managed value for $TMPDIR and having chromium + tests respect it. NSTemporaryDirectory takes its value directly from a non-overridable confstr(_CS_DARWIN_USER_TEMP_DIR) call internally. This CL brings OS X more in line with Windows and Linux which both respect an environment variable ($TEMP and $TMPDIR respectively). BUG= 696803 Review-Url: https://codereview.chromium.org/2713293002 Cr-Original-Commit-Position: refs/heads/master@{#453835} Committed: https://chromium.googlesource.com/chromium/src/+/7b5db573b3ebbc32ee4608163b9ba7e68750ee9a Review-Url: https://codereview.chromium.org/2713293002 Cr-Commit-Position: refs/heads/master@{#453873} [modify] https://crrev.com/95fcf47ca01054ffe38e8609525a6ca8579b7824/base/files/file_util_mac.mm [modify] https://crrev.com/95fcf47ca01054ffe38e8609525a6ca8579b7824/chrome/browser/file_select_helper_mac.mm [modify] https://crrev.com/95fcf47ca01054ffe38e8609525a6ca8579b7824/remoting/host/mac/me2me_preference_pane.mm
,
Mar 1 2017
Second time's the charm, this seems to have stuck.
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/2f339fae3c3cb533e784ce0c1e9a5546a5766ace commit 2f339fae3c3cb533e784ce0c1e9a5546a5766ace Author: Robert Iannucci <iannucci@chromium.org> Date: Wed Mar 01 17:52:12 2017 [run_slave.py] Add TMPDIR to the list of allowed envvars on !windows. BUG= 696803 Change-Id: I279e192c520fb5d6b77d3a00e81f79019a4f88cb Reviewed-on: https://chromium-review.googlesource.com/448162 Reviewed-by: Vadim Shtayura <vadimsh@chromium.org> Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Robbie Iannucci <iannucci@chromium.org> [modify] https://crrev.com/2f339fae3c3cb533e784ce0c1e9a5546a5766ace/slave/run_slave.py
,
Apr 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16abc4a0f8b419fe03dcfba8799cae52f28993b1 commit 16abc4a0f8b419fe03dcfba8799cae52f28993b1 Author: iannucci <iannucci@chromium.org> Date: Mon Apr 03 20:04:32 2017 [base/files] Respect MAC_CHROMIUM_TMPDIR instead of TMPDIR on macOS. Some folks are currently setting $TMPDIR to very long values on macOS, which causes socket name length issues when using $TMPDIR. R=mattm@chromium.org BUG= 698759 , 696803 Review-Url: https://codereview.chromium.org/2770753006 Cr-Commit-Position: refs/heads/master@{#461515} [modify] https://crrev.com/16abc4a0f8b419fe03dcfba8799cae52f28993b1/base/files/file_util_mac.mm [modify] https://crrev.com/16abc4a0f8b419fe03dcfba8799cae52f28993b1/chrome/browser/extensions/test_extension_dir.cc
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc1487fed94d1b5989094220dbbbb80c05841b51 commit fc1487fed94d1b5989094220dbbbb80c05841b51 Author: Matt Mueller <mattm@chromium.org> Date: Mon May 08 21:56:46 2017 [base/files] Respect MAC_CHROMIUM_TMPDIR instead of TMPDIR on macOS. Some folks are currently setting $TMPDIR to very long values on macOS, which causes socket name length issues when using $TMPDIR. R=mattm@chromium.org BUG= 698759 , 696803 Review-Url: https://codereview.chromium.org/2770753006 Cr-Commit-Position: refs/heads/master@{#461515} (cherry picked from commit 16abc4a0f8b419fe03dcfba8799cae52f28993b1) Review-Url: https://codereview.chromium.org/2867943002 . Cr-Commit-Position: refs/branch-heads/3029@{#823} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/fc1487fed94d1b5989094220dbbbb80c05841b51/base/files/file_util_mac.mm [modify] https://crrev.com/fc1487fed94d1b5989094220dbbbb80c05841b51/chrome/browser/extensions/test_extension_dir.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by iannucci@chromium.org
, Feb 28 2017