Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Update html build script subprocess.call #132

Merged

Conversation

bmanifold
Copy link
Contributor

First off, thank you for creating this framework, it's been a ton of fun to use.

I, and a few others working on a small project together, came across this bug while trying to create some custom fields in the configuration manager page. We were running in to a bug where the npm commands in the preBuildHTML.py script weren't getting run as intended.

We've been using PlatformIO to compile (on both Mac and Linux).
PlatformIO: Core-5.2.3

The versions of python we've been using:
Mac - python:3.8.2
Linux - python:3.7.3

Below is a copy of the commit message to give more detail on the changes. Please let me know if any more information is needed/wanted and thank you for considering this PR.

Why:

  • While trying to rebuild the HTML for custom attributes, the
    preBuildHTML.py script was failing when calls were made to
    subprocess.call. Under the hood the subprocess.call function
    calls Popen. The following is from the documentation page of the
    subprocess library:

    https://docs.python.org/3/library/subprocess.html#subprocess.Popen

    If shell is True, it is recommended to pass args as a string rather than as a sequence.
    

    While the documentation says that passing a sequence of program
    arguments should work as intended, building on linux and Mac both
    failed when using the sequence and succeeded when using a single
    string.

This change addresses the need by:

  • Updating all calls to subprocess.call in preBuildHTML.py to use
    single string instead of list as the first argument.

Why:

* While trying to rebuild the HTML for custom attributes, the
  preBuildHTML.py script was failing when calls were made to
  `subprocess.call`.  Under the hood the `subprocess.call` function
  calls `Popen`.  The following is from the documentation page of the
  subprocess library:

  https://docs.python.org/3/library/subprocess.html#subprocess.Popen
  ```
  If shell is True, it is recommended to pass args as a string rather than as a sequence.
  ```

  While the documentation says that passing a sequence of program
  arguments should work as intended, building on linux and Mac both
  failed when using the sequence and succeeded when using a single
  string.

This change addresses the need by:

* Updating all calls to `subprocess.call` in preBuildHTML.py to use
  single string instead of list as the first argument.
@maakbaas
Copy link
Owner

Thanks for the detailed PR, I've merged it into the repository

@maakbaas maakbaas closed this Dec 27, 2021
@maakbaas maakbaas reopened this Dec 27, 2021
@maakbaas maakbaas merged commit ff983a4 into maakbaas:master Dec 27, 2021
@bmanifold bmanifold deleted the buxfix/preBuildHTML-subprocess-call branch December 30, 2021 19:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants