Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execute command from :output-to directory #69

Closed
wants to merge 2 commits into from

Conversation

crisptrutski
Copy link
Contributor

A bit rough, but served for me to test that this would fix :none builds via the Boot tooling.

Perhaps takingdoo-opts for this directly would be more appropriate? This would support more exotic paths (eg. dev build of Nashorn)

Also perhaps this behaviour of automatically using the compile directory could be an option.

Refs #68

@danielcompton
Copy link
Contributor

Are you able to check why the tests are failing now? I'll also need to check this on Windows once the tests are passing.

@crisptrutski
Copy link
Contributor Author

@danielcompton looks like two reasons for failures with examples.

  1. Some runners are installed locally, so need to run from project directory (eg. We tried running ./node_modules/.bin/slimerjs but we couldn't find it your system.)
  2. Relative paths are not being rebased on new root, so you end up with portions doubled (eg. Error: Cannot find module '/home/ubuntu/doo/example/out/out/testable.js')

For (1) there really is no solution but closing http://dev.clojure.org/jira/browse/CLJS-1444. For the test suite we could disable the directory-switch behaviour for those tests, or create global installs.

For (2) we can do that rebase simply enough from the Clojure side

Will look into the library failures now

@danielcompton
Copy link
Contributor

What's the root purpose of this patch? I'm not quite understanding what we're trying to achieve.

@danielcompton
Copy link
Contributor

This might be helpful for executing a command from the context of another directory? http://stackoverflow.com/questions/6811522/changing-the-working-directory-of-command-from-java

@crisptrutski
Copy link
Contributor Author

The mechanism in this PR seems to work well enough - my boot-cljs-test projects start working with it. Happy to fix up its interaction with test suite, mostly opting out for runner commands using paths relative to project.

But this approach is just a band aid, and perhaps a bit extreme in that case. Alluded to a less dramatic PR in the issue, should I submit that too?

@@ -37,13 +37,19 @@
(.close r)
(.toString out))))

(defn exec! [cmd]
(defn exec* [base-dir command-arr]
(if base-dir
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what I proposed is what you've already done here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants