OpenCyphal / pycyphal

Python implementation of the Cyphal protocol stack.
https://pycyphal.readthedocs.io/
MIT License
117 stars 105 forks source link

Ignore empty entries in CYPHAL_PATH #278

Closed pavel-kirienko closed 1 year ago

pavel-kirienko commented 1 year ago

Reported by Charles Cross, copypasta below:


I was able to recreate the issue with this script:

#!/bin/bash
set -e

# Install yakut
pip3 install --user yakut

export CYPHAL_DIR="${HOME}/.cyphal"
mkdir -p ${CYPHAL_DIR} || true

# Delete any previous downloaded or compiled DSDL files
rm -rf ${HOME}/.pycyphal
rm -rf ${CYPHAL_DIR}/*

# Install DSDL files
wget https://github.com/OpenCyphal/public_regulated_data_types/archive/refs/heads/master.zip -O dsdl.zip
unzip dsdl.zip -d ${PWD}
mv -f ${PWD}/public_regulated_data_types*/* ${CYPHAL_DIR}

# Add vendor-specific namespaces the same way, if you need any:
wget https://github.com/Zubax/zubax_dsdl/archive/refs/heads/master.zip -O dsdl.zip
unzip dsdl.zip -d ${PWD}
mv -f ${PWD}/zubax_dsdl*/* ${CYPHAL_DIR}

# Create a sub-directory to put the problematic file in
mkdir -p ardupilot || true

# Create a uavcan DSDL file that causes the error
echo "# Description
uavcan.Timestamp timestamp" > ardupilot/20790.MyType.uavcan

# Set a fresh CYPHAL_PATH
unset CYPHAL_PATH
export CYPHAL_PATH="${CYPHAL_DIR}:${CYPHAL_PATH}"

# Run a yakut command which will trigger automatic DSDL compilation
yakut accommodate

From writing this, I think I figured out the underlying issue. If you don't already have a CYPHAL_PATH env var set, and if you set it with the line in the yakut readme, you end up with a trailing ":" in CYPHAL_PATH, and it appears that the empty righthand side of the delimiter gets interpreted as "cwd" by yakut/pycyphal (not sure which) when DSDL files are compiled.

If you set it without the trailing :, the CWD is not crawled.

maksimdrachov commented 1 year ago

Running the script as-is (source issue-setup.sh):

image

image

maksimdrachov commented 1 year ago

Looking at where CYPHAL_PATH is imported: pycyphal/dsdl/_import_hook.py:104

def get_default_lookup_dirs() -> Sequence[str]:
    return os.environ.get("CYPHAL_PATH", "").replace(os.pathsep, ";").split(";")

Now, trying to this step with the trailing :

image

And without the trailing :

image

Just need to filter out the empty entry?

pavel-kirienko commented 1 year ago

Just need to filter out the empty entry?

Yes.

maksimdrachov commented 1 year ago

https://github.com/OpenCyphal/pycyphal/pull/285