colcon / colcon-hardware-acceleration

An extension for colcon-core to include embedded and Hardware Acceleration capabilities
Apache License 2.0
5 stars 5 forks source link

Address invalid escape sequences in strings #21

Closed cottsay closed 9 months ago

cottsay commented 9 months ago

The backslash character is an escape sequence in Python strings. These strings contain backslashes which do not intend to escape the following character, but rather include the backslash in the string.

There are two possible resolutions:

  1. Make the string a "raw" string by prefixing it with "r". This effectively makes backslashes into a normal character.
  2. Escape the backslashes with another backslash.

I used both approaches in this change where appropriate.

Should resolve:

install/lib/python3.12/site-packages/colcon_hardware_acceleration/subverb/__init__.py:411: SyntaxWarning: invalid escape sequence '\|'
  cmd = "fdisk -l " + rawimage_path + " | grep 'Units\|Unidades' | awk '{print $8}'"
install/lib/python3.12/site-packages/colcon_hardware_acceleration/subverb/__init__.py:685: SyntaxWarning: invalid escape sequence '\|'
  cmd = "fdisk -l " + rawimage_path + " | grep 'Units\|Unidades' | awk '{print $8}'"
install/lib/python3.12/site-packages/colcon_hardware_acceleration/subverb/__init__.py:924: SyntaxWarning: invalid escape sequence '\$'
  content_etc_profile = """
install/lib/python3.12/site-packages/colcon_hardware_acceleration/subverb/emulation.py:279: SyntaxWarning: invalid escape sequence '\|'
  + " | grep 'Units\|Unidades' | awk '{print $8}'"

(note that I have not tested the functionality of this extension, and there are no tests for it. I only know that it clears the warnings and builds successfully.)

vmayoral commented 9 months ago

Thanks for this. Seems just fine to me. The following tries to present a counter example and fails:

import subprocess

def run(cmd, shell=False, timeout=1):
    proc = subprocess.Popen(
        cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=shell
    )
    try:
        outs, errs = proc.communicate(timeout=timeout)
    except subprocess.TimeoutExpired:
        proc.kill()
        outs, errs = proc.communicate()

    if outs:
        outs = outs.decode("utf-8").strip()
    else:
        outs = None

    if errs and proc.returncode:
        errs = errs.decode("utf-8").strip()
    else:
        errs = None

    return outs, errs

# Command with single backslash (will not behave as intended for logical OR in grep)
cmd_single_backslash = "cat testfile | grep 'Units\|Unidades'"
outs_single, errs_single = run(cmd_single_backslash, shell=True)
print("Single Backslash Output:", outs_single or "No output", "\nError:", errs_single or "No error")

# Command with double backslash (correct for escaping in the shell)
cmd_double_backslash = "cat testfile | grep 'Units\\|Unidades'"
outs_double, errs_double = run(cmd_double_backslash, shell=True)
print("\nDouble Backslash Output:", outs_double or "No output", "\nError:", errs_double or "No error")

with testfile content:

Units: 100
Unidades: 200