Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Do not chdir() to phakefile directory #31

Open
wants to merge 1 commit into from

2 participants

@clue
Collaborator

This is done in order to preserve relative path passed as arguments, as
they're relative to the current working directory.

Consider the directory structure:

project/
|- demo/
|   \- example.txt
\- Phakefile.php

Consider I have a task named test which accepts a single argument file:

  • If I'm in the directory project/, I expect to be able to run phake test file=demo/example.txt
  • If I'm in the directory project/demo, I expect to be able to run phake test file=example.txt

Open for discussion:

  • What was the original rational for changing dirs? (What about rake / make?)
  • Is this considered a BC break, and if so, how should this be handled?
@clue clue Do not chdir() to phakefile directory
This is done in order to preserve relative path passed as arguments, as
they're relative to the current working directory.
b68e43f
@jaz303
Owner

Build systems in general must cd to the directory containing the build file so that any file references within the file itself are resolved predictably and consistently; rake and make certainly both perform a chdir.

If implemented, this feature should either be a) behind a command-line flag or b) set within the Phakefile itself (e.g. a leading comment like // phake-nochdir.

@clue
Collaborator

rake and make certainly both perform a chdir

Alright, admittedly I have little experience with relative paths in either make or rake, and a quick search didn't turn up many results either. So you can confirm this is actually the case?

My assumption was that all paths are relative to the current working directory (as with normal (php) scripts), whereas you're suggesting that they should be relative to the file they're defined in, right? This shouldn't show any obvious effects if you're running phake from the folder where your Phakefile is located, but it differs if you're running if from a sub-folder (say "demo/" in the above example).

Personally, I'm okay with it either way, it's just that latter case was unexpected. So if we keep it that way, we should probably address a few things instead:

  • Add documentation to the readme (unexpected behavior with regards to normal php)
  • Make sure the paths are always relative to the file they're defined in.
    • Currently, the chdir() is only applied if you do not explicitly pass in a phakefile, whereas it is not applied if you pass one.
    • Also, this should be kept in mind with regards to PR #16 (splitting into task folders)
  • Store the original working directory somewhere (Application::get_original_working_directory()?) in case you need to resolve a file relative to the current working directory (see above example for passing in relative paths as arguments).
@jaz303
Owner

I've tested the behaviour across make and rake:

  • make in fact only searches the cwd for a makefile, and does follow up with a recursive search of parent directories. If a Makefile path is provided explicitly via the -f option, the working directory is unchanged.
  • rake will chdir to the Rakefile's directory when it is found implicitly. When the -f option is used, the working directory is again unchanged.
@clue
Collaborator

Thanks for taking the time to confirm this!

When the -f option is used, the working directory is again unchanged.

So in other words, using relative paths is unpredictable as it depends on several factors not in control of the Phakefile author? If that is actually the case, I'm even more tempted to say that using chdir() is a bad idea and should be avoided.

In PHP, one usually works around this issue by using paths like require __DIR__ . '/myfile.php' instead. This makes it explicit on when to use relative and when to use absolute paths. Some quick searches seem to indicated this is applicable to rake/ruby as well?

If so, it's probably easier (and safer) to refrain from trying to work around relative paths as per the original patch.

Also, this would open up a few possibilities like

  • Using URLs like phake -f http://mycorporation.local/Phakefile.php
  • Embedded phake and a Phakefile in a single Phar (where __DIR__ looks like phar:///my/path/to/phake.phar/example and there's no way to fake a chdir() within a phar archive)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 18, 2013
  1. @clue

    Do not chdir() to phakefile directory

    clue authored
    This is done in order to preserve relative path passed as arguments, as
    they're relative to the current working directory.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 5 deletions.
  1. +1 −5 lib/phake/Bin.php
View
6 lib/phake/Bin.php
@@ -67,11 +67,7 @@ public function execute($args) {
$runfile = $builder->resolve_runfile(getcwd());
$directory = dirname($runfile);
- if (!@chdir($directory)) {
- throw new Exception("Couldn't change to directory '$directory'");
- } else {
- echo "(in $directory)\n";
- }
+ echo "(in $directory)\n";
}
$builder->load_runfile($runfile);
Something went wrong with that request. Please try again.