Skip to content

Commit

Permalink
fixed issue with the Js compiler being unable to use the system tmp d…
Browse files Browse the repository at this point in the history
…irectory, using the one in storage is much safer across different operating systems
  • Loading branch information
luceos committed Jun 12, 2019
1 parent d2674fb commit 54660eb
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Frontend/Compiler/JsCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ protected function save(string $file, array $sources): bool

$this->assetsDir->put($file, implode("\n", $output));

$mapTemp = tempnam(sys_get_temp_dir(), $mapFile);
$mapTemp = tempnam(storage_path('tmp'), $mapFile);
$map->save($mapTemp);
$this->assetsDir->put($mapFile, file_get_contents($mapTemp));
@unlink($mapTemp);
Expand Down

4 comments on commit 54660eb

@franzliedke
Copy link
Contributor

Choose a reason for hiding this comment

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

So does this fix #487?

@luceos
Copy link
Member Author

@luceos luceos commented on 54660eb Jun 12, 2019

Choose a reason for hiding this comment

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

Thanks yes, after your message I went over all instances of core to seek additional references. What I did notice is mixed use of storage_path (which uses DIRECTORY_SEPARATOR) and the Application::storage_path method which allows no arguments. Which do we prefer?

@franzliedke
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to avoid the global helpers where possible.

@luceos
Copy link
Member Author

@luceos luceos commented on 54660eb Jun 16, 2019

Choose a reason for hiding this comment

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

Me too, I guess we should get rid of them soon then. I'll set up an issue for that.

Please sign in to comment.