Download packages to temp dir inside system cache.

Download packages to temp dir inside system cache.

This fixes an issue where the rename will fail if your system temp directory is
on a different volume from your user directory (and thus your pub cache).
Committed: https://code.google.com/p/dart/source/detail?r=13674

[Bob Nystrom, 2012-10-16 02:31:41]

Dan, this fixes an issue John discovered where pub install doesn't work on his
machine with the latest stuff. My patch from earlier today downloads packages to
your system temp directory and then moves it to the pub cache on success.

That's awesome for making sure you don't have busted stuff in your cache. It's
less awesome if your OS temp directory and your pub cache are on different
volumes and the rename fails. :(

So this patch fixes that. It still downloads to a temp directory, but one inside
the system cache itself, so the rename should work.

I don't think many users will be in this situation, so I don't think this needs
to be in M1, but I wanted to have a fix in place as soon as possible.

[kasperl, 2012-10-16 06:08:50]

LGTM.

https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart#newcode224
utils/pub/pub.dart:224: future = future.chain((_) => cache_.deleteTempDir());
I guess this should always be done (also in the exception case), but let's not
mess with that now.

https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart
File utils/pub/system_cache.dart (right):

https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart#n...
utils/pub/system_cache.dart:85: return ensureDir(join(rootDir,
'_temp')).chain((tempDir) {
Rename tempDir variable to avoid shadowing the getter? I'd suggest temp.

Use tempDir instead of join(rootDir, '_temp').

[ricow1, 2012-10-16 06:21:18]

On 2012/10/16 06:08:50, kasperl wrote:
> LGTM.
> 
> https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart
> File utils/pub/pub.dart (right):
> 
> https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart#newcode224
> utils/pub/pub.dart:224: future = future.chain((_) => cache_.deleteTempDir());
> I guess this should always be done (also in the exception case), but let's not
> mess with that now.
> 
> https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart
> File utils/pub/system_cache.dart (right):
> 
>
https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart#n...
> utils/pub/system_cache.dart:85: return ensureDir(join(rootDir,
> '_temp')).chain((tempDir) {
> Rename tempDir variable to avoid shadowing the getter? I'd suggest temp.
> 
> Use tempDir instead of join(rootDir, '_temp').
I landed this with Kaspers suggestion incorporated (I did not upload a new
patchset to rietveld since I am using svn)

[Bob Nystrom, 2012-10-16 14:10:22]

On 2012/10/16 06:21:18, ricow1 wrote:
> On 2012/10/16 06:08:50, kasperl wrote:
> > LGTM.
> > 
> > https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart
> > File utils/pub/pub.dart (right):
> > 
> >
https://codereview.chromium.org/11151035/diff/1/utils/pub/pub.dart#newcode224
> > utils/pub/pub.dart:224: future = future.chain((_) =>
cache_.deleteTempDir());
> > I guess this should always be done (also in the exception case), but let's
not
> > mess with that now.

This is intentional (for now). I wanted to keep the patch as simple as possible.
I tried .onComplete() which should be the right way to do this, but I couldn't
get it to work and that corner of Future isn't well-used.

On the off chance that you do fail and leave stuff in _temp, it will just get
cleaned out the next time around.

> > https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart
> > File utils/pub/system_cache.dart (right):
> > 
> >
>
https://codereview.chromium.org/11151035/diff/1/utils/pub/system_cache.dart#n...
> > utils/pub/system_cache.dart:85: return ensureDir(join(rootDir,
> > '_temp')).chain((tempDir) {
> > Rename tempDir variable to avoid shadowing the getter? I'd suggest temp.

I don't have a strong preference one way or the other, but at least in the pub
code base, we don't tend to worry about shadowing too much.

> > 
> > Use tempDir instead of join(rootDir, '_temp').
> I landed this with Kaspers suggestion incorporated (I did not upload a new
> patchset to rietveld since I am using svn)

Thank you!