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

add autocompile support, controlled via --compile (default) and --no-compile #29

Closed
wants to merge 2 commits into from

Conversation

timbertson
Copy link
Contributor

Discussed on the mailing list a little already, but this PR may be easier to comment / track changes.

@talex5
Copy link
Member

talex5 commented Nov 23, 2014

Just to record a few of the things we discussed so we don't forget:

  • need to support autocompile in GUI mode
  • think about whether --autocompile should be the default (e.g. should existing updates doing background updates start to compile?)
  • fix unit-tests
  • document changes to the selections XML format (not sure mode is a good name; maybe requires_compilation=true or something?)
  • consider using `binary_of source impl_type
  • update 0compile to support this

@timbertson
Copy link
Contributor Author

think about whether --autocompile should be the default (e.g. should existing updates doing background updates start to compile?)

I feel strongly that it should be the default, even in the background. My main intention in adding auto compilation is to be able to distribute source code and have it be usable (including updates).

Making it explicitly opt-in is not much more useful than just running 0compile explicitly, which is sufficiently awkward that nobody does it.

fix unit-tests

I've now fixed the compilation errors, and fixed the trivial selection test case failures. But I'm getting a bunch of test failures like:

Error: 0install:13:selections:2:commands

Fake_system.Would_spawn(_)
Raised at file "tests/fake_system.ml", line 256, characters 19-57

Not really sure what's going on there, as it doesn't seem like the backtrace touches any of my new code (and I'm finding oUnit a little obtuse). I do spawn 0compile when we end up with selections that need compilation, but I wouldn't expect any existing test cases to actually trigger that. Any hints?

@talex5
Copy link
Member

talex5 commented Dec 2, 2014

I've added formatting for these exceptions on master. If you rebase you'll get e.g. (from test_run.ml):

Error: 0install:23:run:2:abs-main

Test tried to spawn process /home/tal/Projects/zero-install/0install/build/ocaml/0install download --refresh --console -v --command run /home/tal/Projects/zero-install/0install/tests/Command.xml

Perhaps your patch makes it decide to try recompiling it instead of using the older binary?

@timbertson
Copy link
Contributor Author

Thanks. I rebased, and you're right, that does seem to be the reason for the failures. e.g:

==============================================================================
Error: 0install:23:run:2:abs-main

Test tried to spawn process /home/tim/dev/0install/zeroinstall/build/ocaml/0install download --refresh --console -v --command run /home/tim/dev/0install/zeroinstall/tests/Command.xml
Raised at file "tests/fake_system.ml", line 265, characters 19-57

I'm haven't been able out how my changes would affect that though.

@talex5
Copy link
Member

talex5 commented Dec 31, 2014

I've been wondering if we should break this work up into smaller pieces:

  1. Clean up the current mess in impl_provider.ml
  2. Implement the current 0compile autocompile behaviour in the OCaml solver, so that 0compile can use that instead of its current hack.
  3. Improve the selections (e.g. to include build dependencies in the selections).
  4. Add a command-line interface for compiling apps/interfaces.
  5. Make autocompiling apps the default.

I've made a start on 1 and 2 here: https://github.com/0install/0install/commits/autocompile

Currently, if you set may_compile = true in the scope filter then it:

  1. Includes source feeds even when looking for binaries.
  2. Converts every source impl it finds into a binary.

It doesn't yet implement the 0compile algorithm (e.g. adding commands and binary dependencies). Does this look like a reasonable direction?

3/4/5 are the features from your patches that aren't currently in mine.

@timbertson
Copy link
Contributor Author

Yeah, this definitely seems like a reasonable approach to me :)

@talex5
Copy link
Member

talex5 commented Jan 10, 2015

OK, 1 and 2 are now done and merged.
I don't think 3 is a good idea, because it greatly increases the number of feeds you need to download to include the build tools and headers for every potential binary, recursively. So, I now just assume every source implementation can be compiled, which is what 0compile does already.

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