Support Python functions as Ninja targets

NiklasRosenstein commented 8 years ago

[ ] Implement the ability to pipe data into the stdin of a Python function invoked with the Craftr daemon
[x] ~~Delayed execution of Craftr modules when the initial execution phase is skipped~~
[ ] Verify that it works safe with parallel requests (assuming the called functions do not perform operations that could influence other requests/threads)
[x] Verify that it works on Windows
[ ] Can we avoid patching sys.stdin, etc. globally in craftr/daemon.py?

I want to be able to run a Python function defined in a Craftfile from ninja. This could be realized with sockets. Eg. the generated ninja rule for a function dance in the Craftr module walz could look like this:

rule walz.do_the_dance
  command = craftr-com walz.dance localhost $CRAFTR_PORT -- $in -o $out
  description = walz.dance()

The craftr-com program would conntect to Craftr via a socket and pass the program arguments. These arguments would be passed as a list to the Python function craftr.ext.walz.dance(). Note that it could be possible that a delayed execution of the Craftr module could be necessary with this technique. (Craftr skips the execution of build definitions if it is only used to invoke ninja with the -b option).

Inside of Craftr, it could look like this:

def dance(args):
  parser = argument.ArgumentParser(prog='dance()')
  parser.add_argument('infiles', args='*')
  parser.add_argument('-o', default=None)
  args = parser.parse_args()
  print(args)
  # xxx: do stuff

do_the_dance = rules.pycom(
  function = dance,
  inputs = path.glob('src/*.c'),
  outputs = ['foo'],
)

winksaville commented 8 years ago

I'm not sure what you want to do exactly, but you might look at what meson is doing, they've got some "magic" code which allows them to rebuild the build.ninja from ninja if the meson.build files change.

On Tue, Dec 29, 2015 at 9:51 AM Niklas Rosenstein notifications@github.com wrote:

I want to be able to run a Python function defined in a Craftfile from ninja. This could be realized with sockets. Eg. the generated ninja rule for a function dance in the Craftr module walz could look like this:

rule walz.do_the_dance command = craftr-com walz.dance localhost $CRAFTR_PORT -- $in -o $out description = walz.dance()

The craftr-com program would conntect to Craftr via a socket and pass the program arguments. These arguments would be passed as a list to the Python function craftr.ext.walz.dance(). Note that it could be possible that a delayed execution of the Craftr module could be necessary with this technique. (Craftr skips the execution of build definitions if it is only used to invoke ninja with the -b option).

Inside of Craftr, it could look like this:

def dance(args): parser = argument.ArgumentParser(prog='dance()') parser.add_argument('infiles', args='*') parser.add_argument('-o', default=None) args = parser.parse_args() print(args)

xxx: do stuff

do_the_dance = rules.pycom( function = dance, inputs = path.glob('src/*.c'), outputs = ['foo'], )

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49.

NiklasRosenstein commented 8 years ago

Say you have some Python script that you want to use as "command" in Ninja. That is easily possible by just using Target(['path/to/script.py', 'args here...'], ...). But there can be cases when this Python script would need additional data from the Craftfile. I want to be able to execute the Python code that is run by Ninja in the same process as the Craftfile.

Keep in mind that Craftr currently really acts like an overlay to ninja, so you'd never run ninja directly. Of course you could still do that. Only if you use that feature described in this issue, it wouldn't be possible since Craftr must be running as well.

winksaville commented 8 years ago

SG, but I do like running ninja directly, as this is how cmake and meson work too.

Anyway, speaking to this feature, it would seem it should be possible to run craftr from ninja and have it turn itself into a daemon if needed, but that just me BS'ing you :)

On Tue, Dec 29, 2015, 9:59 AM Niklas Rosenstein notifications@github.com wrote:

Say you have some Python script that you want to use as "command" in Ninja. That is easily possible by just using Target(['path/to/script.py', 'args here...'], ...). But there can be cases when this Python script would need additional data from the Craftfile. I want to be able to execute the Python code that is run by Ninja in the same process as the Craftfile.

Keep in mind that Craftr currently really acts like an overlay to ninja, so you'd never run ninja directly. Of course you could do that, only if you use that feature described in this issue, it wouldn't be possible since Craftr must be running as well.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-167844371.

NiklasRosenstein commented 8 years ago

SG, but I do like running ninja directly, as this is how cmake and meson too.

Sure you can still do that, it's just more convenient using the Craftr command line as you can use relative target identifiers if you want to rebuild only a specific target and you don't have to switch to the build directory. :)

Anyway, speaking to this feature, it would seem it should be possible to run craftr from ninja and have it turn itself into a daemon if needed, but that just me BS'ing you :)

Good idea, that would probably work as well! craftr-com (or whatever the program to communicate with Craftr via a socket would be called) could spawn a craftr process if it does not yet exist. Not sure how I would manage to create only one background process though, and not for each time craftr-com is called.

winksaville commented 8 years ago

If you use a well known port only one service can use it at a time so creating the socket will fail. Other techniques are possible like sent ng an environment variable.

On Tue, Dec 29, 2015, 10:57 AM Niklas Rosenstein notifications@github.com wrote:

SG, but I do like running ninja directly, as this is how cmake and meson too.

Sure you can still do that, it's just more convenient using the Craftr command line as you can use relative target identifiers if you want to rebuild only a specific target. :)

Anyway, speaking to this feature, it would seem it should be possible to run craftr from ninja and have it turn itself into a daemon if needed, but that just me BS'ing you :)

Good idea, that would probably work as well! craftr-com (or whatever the program to communicate with Craftr via a socket would be called) could spawn a craftr process if it does not yet exist. Not sure how I would manage to create only one background process though, and not for each time craftr-com is called.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-167853265.

NiklasRosenstein commented 8 years ago

I've added an entry in the Wiki: https://github.com/craftr-build/craftr/wiki/Python-Tools

winksaville commented 8 years ago

Cool, I think this is a great feature!!!

Clarification, in the documentation you give an example, but not being that familiar with craftr yet the example is somewhat opaque. I assume the "magic" is "rules.pyfunc" (I'd like to suggest you mention that explicitly in the text, assuming I'm correct). You might also move the example up and mention the "exceptions" afterwards. Although you might mention early in the text there are "exceptions".

One other thing you say it won't work with the "-b" only but you haven't really documented the options anywhere that I saw. In the Home page you mention "-b" and "-e" in passing but imply there are other options. Anyway, if I use the export option "-e" I can use ninja without running craftr, correct? If I do that do rules.pyfunc's work or is that the same as using "-b" only and thus doesn't work?

On Tue, Dec 29, 2015 at 9:33 PM Niklas Rosenstein notifications@github.com wrote:

I've added an entry in the Wiki: https://github.com/craftr-build/craftr/wiki/Call-Python-functions-from-Ninja

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-167939915.

NiklasRosenstein commented 8 years ago

Thanks Wink, I value your feedback on the documentation. Writing documentation is a lot of work. ●︿●

The magic is not in rules.pyfunc(), it simply creates a Target with the command python -m craftr.daemon. The actual magic is in the craftr.daemon module that implements a socket server and client. I'll try to make the documentation on this topic more clear!

By the way, I just updated the Wiki with a lot of information. Especially the "Getting Started" part, although it is not very complete yet. I should probably also add a "Quickstart" tutorial as the "Getting Started" guide really focuses on the basics and I want it to explain the system from the ground up.

Regarding the command-line options, there's now the "Command-line" section in the "Getting Started" page that gives a little of information about -e and -b. I would like to avoid out-of-date information as much as possible and the command-line options change frequently atm. You can run craftr --help or -h to show a list of all available command line options with a short explanation.

if I use the export option "-e" I can use ninja without running craftr, correct? If I do that do rules.pyfunc's work or is that the same as using "-b" only and thus doesn't work?

Exactly, though not with the rules.pyfunc() feature. That's exactly the limitation the Wiki page was "talking" about. There would be a way to make make it work, but it would be very troublesome.

Start craftr --daemon localhost:1234 in a separate shell
cd build
export CRAFTR_DAEMON_URI=localhost:1234
ninja

NiklasRosenstein commented 8 years ago

Btw. while testing your baremetal-hi repository, I used this feature to dynamically generate a linker script with the correct path to the startup object file.

def render_ldscript(args):
  parser = argparse.ArgumentParser(prog='render_ldscript')
  parser.add_argument('input', type=argparse.FileType('r'))
  parser.add_argument('output', type=argparse.FileType('w'))
  args = parser.parse_args(args)

  content = args.input.read()
  content = content.replace('$_STARTUP_OBJ_$', startup_object.outputs[0])
  args.output.write(content)
  print("rendered!")

# Target that renders the LD script with the corret path to the
# startup_object output filename.
testelf_ldscript = rules.pyfunc(
  render_ldscript,
  args = ['$in', '$out'],
  inputs = [path.local('link.craftr.ld')],
  outputs = 'link.ld',
)

I still feel like its a bit tedious with all the arguments that must be passed to rules.pyfunc(). I think a class could 1. implement the Python function that will be called from Ninja and 2. wrap it in a nice rule interface and hide the low-level craftr.Target() interface.

winksaville commented 8 years ago

I agree, documentation sucks :)

Could you create a pull request for baremetal-hi? Also, did you get vendor-install-tools working and do you have a pull request for that?

On Wed, Dec 30, 2015 at 10:32 AM Niklas Rosenstein notifications@github.com wrote:

Btw. while testing your baremetal-hi repository, I used this feature to dynamically generate a linker script with the correct path to the startup object file.

def render_ldscript(args): parser = argparse.ArgumentParser(prog='render_ldscript') parser.add_argument('input', type=argparse.FileType('r')) parser.add_argument('output', type=argparse.FileType('w')) args = parser.parse_args(args)

content = args.input.read() content = content.replace('$_STARTUPOBJ$', startup_object.outputs[0]) args.output.write(content) print("rendered!")

Target that renders the LD script with the corret path to the# startup_object output filename.

testelf_ldscript = rules.pyfunc( render_ldscript, args = ['$in', '$out'], inputs = [path.local('link.craftr.ld')], outputs = 'link.ld', )

I still feel like its a bit tedious with all the arguments that must be passed to rules.pyfunc(). I think a class could 1. implement the Python function that will be called from Ninja and 2. wrap it in a nice rule interface and hide the low-level craftr.Target() interface.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-168050212.

NiklasRosenstein commented 8 years ago

baremetal-hi

Unfortunately I still get that crt0.o could not be found error. :disappointed:

niklas@sunbird:~/Desktop/baremetal-hi$ craftr -ebcc
Cleaning... 3 files.
[3/5] arm-eabi-gcc -x c -c /home/niklas/Desktop/baremetal-hi...26ej-s -nodefaultlibs -nostdlib -nostartfiles -ffreestanding
craftr| info|:daemon -> [127.0.0.1:46780] connection accepted
craftr| info|:daemon -> [127.0.0.1:46780] @@ baremetal_hi.testelf_ldscript()
[3/5] /usr/bin/python3 -m craftr.daemon baremetal_hi.testelf...nk.craftr.ld /home/niklas/Desktop/baremetal-hi/build/link.ld
rendered!
[4/5] arm-eabi-gcc  -Wl,-T,link.ld -o /home/niklas/Desktop/baremetal-hi/build/test.elf
FAILED: arm-eabi-gcc  -Wl,-T,link.ld -o /home/niklas/Desktop/baremetal-hi/build/test.elf
/home/niklas/opt/lib/gcc/arm-eabi/5.2.0/../../../../arm-eabi/bin/ld: cannot find crt0.o: No such file or directory
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
niklas@sunbird:~/Desktop/baremetal-hi$
niklas@sunbird:~/Desktop/baremetal-hi$ cat build/link.ld
ENTRY(_Reset)
SECTIONS
{
  . = 0x10000;
  .startup  . : { baremetal_hi/obj/startup.o(.text) }
  .text : { *(.text) }
  .data : { *(.data) }
  .bss : { *(.bss COMMON) }
  . = ALIGN(8);
  . = . + 0x1000; /* 4kB stack */
  stack_top = .;
}

vendor-install-tools

Although it works on Travis CI (https://travis-ci.org/NiklasRosenstein/vendor-install-tools/builds/99310152), I still can't install it here on Ubuntu. Still the same error, yet libtool is installed. :sleepy:

niklas@sunbird:~/Desktop/vendor-install-tools$ ./install crosstool-ng
installing crosstools-ng 1.22.0 ...
Cloning into '/home/niklas/tmp/crosstools-ng'...
remote: Counting objects: 1208, done.
remote: Compressing objects: 100% (794/794), done.
remote: Total 1208 (delta 433), reused 939 (delta 393), pack-reused 0
Receiving objects: 100% (1208/1208), 1.92 MiB | 1.74 MiB/s, done.
Resolving deltas: 100% (433/433), done.
Checking connectivity... done.
Note: checking out 'ced321f73c8373f4d3212bcaede7de073e3e27fe'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

Running autoconf...
Done. You may now run:
    ./configure
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking for a BSD-compatible install... /usr/bin/install -c
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for a sed that does not truncate output... /bin/sed
checking whether sed understands -r -i -e... yes
checking whether ln -s works... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking how to run the C preprocessor... gcc -E
checking for ranlib... ranlib
checking for gobjcopy... no
checking for objcopy... objcopy
checking for absolute path to objcopy... /usr/bin/objcopy
checking for gobjdump... no
checking for objdump... objdump
checking for absolute path to objdump... /usr/bin/objdump
checking for greadelf... no
checking for readelf... readelf
checking for absolute path to readelf... /usr/bin/readelf
checking for gperf... gperf
checking for absolute path to gperf... /usr/bin/gperf
checking for bison... bison
checking for flex... flex
checking for makeinfo... makeinfo
checking for cut... cut
checking for stat... stat
checking for readlink... readlink
checking for wget... wget
checking for tar... tar
checking for gzip... gzip
checking for bzip2... bzip2
checking for help2man... help2man
checking for gpatch... no
checking for patch... patch
checking for absolute path to patch... /usr/bin/patch
checking for bash >= 3.1... /bin/bash
checking for GNU awk... /usr/bin/gawk
checking for GNU make >= 3.80... /usr/bin/make
checking whether /usr/bin/make sets $(MAKE)... yes
checking for make 3.81... no
checking for GNU libtool >= 1.5.26... no
configure: error: could not find GNU libtool >= 1.5.26
Traceback (most recent call last):
  File "./install", line 26, in <module>
    installlib.main()
  File "/home/niklas/Desktop/vendor-install-tools/installlib.py", line 341, in main
    install(name, **curr_options)
  File "/home/niklas/Desktop/vendor-install-tools/installlib.py", line 279, in install
    runpy.run_path(installer['script'], {'settings': settings}, run_name='__main__')
  File "/usr/lib/python2.7/runpy.py", line 240, in run_path
    return _run_module_code(code, init_globals, run_name, path_name)
  File "/usr/lib/python2.7/runpy.py", line 82, in _run_module_code
    mod_name, mod_fname, mod_loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "installers/crosstool-ng.py", line 59, in <module>
  File "installers/crosstool-ng.py", line 53, in main
  File "/home/niklas/Desktop/vendor-install-tools/installlib.py", line 133, in run
    result.check_returncode()
  File "/home/niklas/Desktop/vendor-install-tools/installlib.py", line 87, in check_returncode
    raise CalledProcessError(self.returncode, self.args, self.stdout, self.stderr)
installlib.CalledProcessError: Command ['./configure', '--enable-local', '--prefix=/home/niklas/opt'] exited with non-zero exit status 1

NiklasRosenstein commented 8 years ago

Sometimes you just need to try it the next day and you'll find better keywords for Google :) I need the libtool-bin package. Installing crosstool-ng works now. Found the solution here: https://github.com/pfalcon/esp-open-sdk/issues/58

Unfortunately I can not create a pull request as the branches have no commits in common. I could delete everything from master, then merge, if that's what you want.

winksaville commented 8 years ago

Glad you've made progress. Please do a merge, I'd rather you not delete the crosstool-ng work, but if you have crosstool-ng on your branch then whatever is easy is fine.

On Wed, Dec 30, 2015 at 11:44 AM Niklas Rosenstein notifications@github.com wrote:

Sometimes you just need to try it the next day and you'll find better keywords for Google :) I need the libtool-bin package. Installing crosstool-ng works now.

Unfortunately I can not create a pull request as the branches have no commits in common. I could delete everything from master, then merge, if that's what you want.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-168062708.

NiklasRosenstein commented 8 years ago

I'll submit a pull request soon. Regarding the installation of a cross-compiler with crosstool-ng, I get

[INFO ]  Performing some trivial sanity checks
[INFO ]  Build started 20151231.021149
[INFO ]  Building environment variables
[INFO ]  =================================================================
[INFO ]  Retrieving needed toolchain components' tarballs
[INFO ]  Retrieving needed toolchain components' tarballs: done in 110.05s (at 01:55)
[INFO ]  =================================================================
[INFO ]  Extracting and patching toolchain components
[INFO ]  Extracting and patching toolchain components: done in 165.00s (at 04:40)
[INFO ]  =================================================================
[INFO ]  Installing GMP for host
[INFO ]  Installing GMP for host: done in 104.73s (at 06:25)
[INFO ]  =================================================================
[INFO ]  Installing MPFR for host
[INFO ]  Installing MPFR for host: done in 76.38s (at 07:42)
[INFO ]  =================================================================
[INFO ]  Installing ISL for host
[INFO ]  Installing ISL for host: done in 75.38s (at 08:57)
[INFO ]  =================================================================
[INFO ]  Installing MPC for host
[INFO ]  Installing MPC for host: done in 38.57s (at 09:36)
[INFO ]  =================================================================
[INFO ]  Installing binutils for host
[ERROR]    make[3]: *** [configure-ld] Error 1
[ERROR]    make[2]: *** [all] Error 2
[ERROR]   
[ERROR]  >>
[ERROR]  >>  Build failed in step 'Installing binutils for host'
[ERROR]  >>        called in step '(top-level)'
[ERROR]  >>
[ERROR]  >>  Error happened in: CT_DoExecLog[scripts/functions@257]
[ERROR]  >>        called from: do_binutils_backend[scripts/build/binutils/binutils.sh@240]
[ERROR]  >>        called from: do_binutils_for_host[scripts/build/binutils/binutils.sh@105]
[ERROR]  >>        called from: main[scripts/crosstool-NG.sh@646]
[ERROR]  >>
[ERROR]  >>  For more info on this error, look at the file: 'build.log'
[ERROR]  >>  There is a list of known issues, some with workarounds, in:
[ERROR]  >>      '/home/niklas/opt/share/doc/crosstool-ng/crosstool-ng-1.22.0/B - Known issues.txt'
[ERROR]   
[ERROR]  (elapsed: 10:45.09)

Did you experience this as well?

NiklasRosenstein commented 8 years ago

Could you grant me rights to contribute to the baremetal-hi repository? That way I don't have to fork it. Same would be useful for vendor-install-tools. I can keep changes on a separate branch and still create pull requests.

NiklasRosenstein commented 8 years ago

Btw. also a very important point on manually invoking ninja: The environment variables could be set up in a .craftrc file will not be available to you and you have to set them manually every time. I use it in baremetal-hi to add ~/opt/bin to PATH and set CC to arm-eabi-gcc. My actual bash environment stays clean from these variables.

NiklasRosenstein commented 8 years ago

I needed to pass the flags (-nodefaultlibs, etc.) to the linker as well :sweat_smile: I can compile baremeta-hi now!

$ craftr -eb run
[1/1] qemu-system-arm -M versatilepb -m 128M -no-reboot -kernel /home/niklas/Desktop/baremetal-hi/build/baremetal_hi/obj/testbin
audio: Could not init `oss' audio driver
VNC server running on '127.0.0.1:5900'

winksaville commented 8 years ago

Bravo, I'll grant you the necessary rights. Did you workout the crosstool-ng problem. I had some problems like you did and in general the build.log helped find a solution.

On Wed, Dec 30, 2015 at 8:24 PM Niklas Rosenstein notifications@github.com wrote:

I needed to pass the flags (-nodefaultlibs, etc.) to the linker as well [image: :sweat_smile:] I can compile baremeta-hi now!

[1/1] qemu-system-arm -M versatilepb -m 128M -no-reboot -kernel /home/niklas/Desktop/baremetal-hi/build/baremetal_hi/obj/testbin audio: Could not init `oss' audio driver VNC server running on '127.0.0.1:5900'

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-168123409.

winksaville commented 8 years ago

Added you as a collaborator to vendor-install-tools and baremetal-hi.

On Thu, Dec 31, 2015 at 11:14 AM Wink Saville wink@saville.com wrote:

Bravo, I'll grant you the necessary rights. Did you workout the crosstool-ng problem. I had some problems like you did and in general the build.log helped find a solution.

On Wed, Dec 30, 2015 at 8:24 PM Niklas Rosenstein < notifications@github.com> wrote:

I needed to pass the flags (-nodefaultlibs, etc.) to the linker as well [image: :sweat_smile:] I can compile baremeta-hi now!

[1/1] qemu-system-arm -M versatilepb -m 128M -no-reboot -kernel /home/niklas/Desktop/baremetal-hi/build/baremetal_hi/obj/testbin audio: Could not init `oss' audio driver VNC server running on '127.0.0.1:5900'

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/49#issuecomment-168123409 .

NiklasRosenstein commented 8 years ago

Delayed loading won't be easily possible. Instead, a .craftr-rts file is now created if the RTS feature is required. If that file exists in the build directory, Craftr will not skip the execution phase even if possible and make sure the RTS is up and running like with the -e option.

NiklasRosenstein commented 8 years ago

I'll close this issue as the main feature is implemented and works and move the Todo's from the initial post to #70

craftr-build / craftr-build-4.x

Support Python functions as Ninja targets #49

xxx: do stuff

Target that renders the LD script with the corret path to the# startup_object output filename.

baremetal-hi

vendor-install-tools