olofk / fusesoc

Package manager and build abstraction tool for FPGA/ASIC development
BSD 2-Clause "Simplified" License
1.19k stars 245 forks source link

Hook scripts are called from wrong directory when part of a fileset #353

Open imphil opened 4 years ago

imphil commented 4 years ago

Have a look at this core file defining a hook and an associated fileset.

CAPI=2:
# Copyright lowRISC contributors.
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
# SPDX-License-Identifier: Apache-2.0
name: "lowrisc:ibex:check_tool_requirements:0.1"
description: "Check tool requirements"

filesets:
  files_check_tool_requirements:
    files:
      - ./util/check_tool_requirements.py
      - ./tool_requirements.py

scripts:
  check_tool_requirements:
    cmd: 
      - python3 
      - util/check_tool_requirements.py
    filesets:
      - files_check_tool_requirements

targets:
  default:
    hooks:
      pre_build:
        - tool_verilator ? (check_tool_requirements)

When executing, we get the following:

DEBUG: Working directory: /home/philipp/src/ibex/build/lowrisc_ibex_ibex_core_tracing_0.1/lint-verilator
python3: can't open file 'util/check_tool_requirements.py': [Errno 2] No such file or directory

The problem:

Things are made worse by the fact that copyto on filesets included as part of a script are ignored.

@olofk has this ever been working, and if so, how? Or how would you expect this to work?

olofk commented 4 years ago

Hmm.. I'm not sure I understand what's going on. I have used scripts successfully in the past. One example is in LED to Believe. Check for the proginfo scripts.

One difference is that I'm not using filesets in the scripts section in my case. I honestly can't remember why the filesets were necessary in the first place

imphil commented 4 years ago

The proginfo ones work because you create a dependency on the fileset in the targets section, not in the scripts section, and use copyto (https://github.com/fusesoc/blinky/blob/master/blinky.core#L50) to copy the script into the WORKDIR. That's the workaround I'm using as well.

A fix for this one will be tricky while keeping backwards compat, I'll need to think about a bit more.

olofk commented 4 years ago

I wish I could remember how this was supposed to work. Why do you even need to specify filesets in the scripts section and not just include them in the target. I have a vague memory that there was a good reason for it.

But also, if we come up with a better solution that is not backwards compatible we could create a new section (hooks?) and deprecate the scripts section eventually