Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add IDF_SKIP_PYTHON_CHECK env var (IDFGH-7790) #9328

Closed
wants to merge 1 commit into from

Conversation

stkw0
Copy link
Contributor

@stkw0 stkw0 commented Jul 11, 2022

In some environments IDF is installed system-wide. With v5.0 some checks are done against user directories which obviously doesn't contain the required dependencies (they are installed system-wide) and as a result IDF fails to execute. A workaround for this is to simply ignore those checks if the user knows those dependencies are really provided, this way IDF can be run successfully.

Another approach would be to correctly check if dependencies are met wherever they are located.

See #9327

@CLAassistant
Copy link

CLAassistant commented Jul 11, 2022

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 11, 2022
@github-actions github-actions bot changed the title add IDF_SKIP_PYTHON_CHECK env var add IDF_SKIP_PYTHON_CHECK env var (IDFGH-7790) Jul 11, 2022
@igrr
Copy link
Member

igrr commented Jul 11, 2022

@stkw0 If you create a global (not user-specific) virtual environment and add the environment's interpreter to PATH, do you still get this error? I think it should work without bypassing the check.

Something along these lines:

python -m venv /path/to/global/venv
export PATH=/path/to/global/venv/bin:$PATH
python -m pip install -r $IDF_PATH/tools/requirements/requirements.core.txt -c /path/to/espidf.constraints.v5.0.txt
# now the python environment is set up and added to your PATH, it should be possible to run idf.py

@stkw0
Copy link
Contributor Author

stkw0 commented Jul 11, 2022

Right now I have IDF installed as any other application (say firefox, or whatever), the package managers is the one that installs IDF, this behavior is working fine in version 4.x of IDF. To avoid breaking this behavior no venv can be used as this would require user set-up (which is unfeasible because the package manager runs in a different user). After the package managers installs IDF it should be working without any configuration from user perspective (that's how it's working with v4.x and the idea would be to still work in a similar manner in 5.0)

I understand that IDF is mainly used through local user installation, but is unfeasible in some use cases. I'm sorry if my explanations are not clear enough.

@igrr
Copy link
Member

igrr commented Jul 11, 2022

Do you mean that you install all the Python package dependencies of ESP-IDF globally (as in, sudo pip install ...)?

If that is the case, I'm afraid this won't work in general, since IDF's dependencies may conflict that with dependencies of other software in the system. There will also be a problem if multiple versions of IDF are installed side by side on the same system. With our current installation approach, each version will use its own virtual environment, and as a result can use different versions of packages.

To avoid breaking this behavior no venv can be used as this would require user set-up (which is unfeasible because the package manager runs in a different user)

I'm afraid I don't understand the reason behind this. It is possible to create a virtual environment as one user and then use it as another user. (For example, you can do python -m venv /opt/my-venv && /opt/my-venv/bin/python -m pip install ... as one user and then use this virtual environment as another user.)

@igrr
Copy link
Member

igrr commented Jul 11, 2022

ESP-IDF install scripts also support an environment variable, IDF_TOOLS_PATH. If not set, it is assumed to be $HOME/.espressif. However you can set it to another location, outside of user's home directory.

For example:

export IDF_TOOLS_PATH=/opt/esp
$IDF_PATH/install.sh

This will install all the tools and the virtual environment to /opt/esp.

Later, as a regular user, you can run $IDF_PATH/export.sh and it will pick up the tools installed in /opt/esp. The only requirement is that IDF_TOOLS_PATH should be set to the same value in the user's environment.

@stkw0
Copy link
Contributor Author

stkw0 commented Jul 11, 2022

I mean more like sudo apt install idf. The package contains the dependencies of IDF and "apt" installs all dependencies. The problem of conflict between dependencies will then relay on the package manager itself, in some cases it would be no problem to have different versions of IDF at the same time.

@stkw0
Copy link
Contributor Author

stkw0 commented Jul 11, 2022

ESP-IDF install scripts also support an environment variable, IDF_TOOLS_PATH. If not set, it is assumed to be $HOME/.espressif. However you can set it to another location, outside of user's home directory.

For example:

export IDF_TOOLS_PATH=/opt/esp
$IDF_PATH/install.sh

This will install all the tools and the virtual environment to /opt/esp.

Later, as a regular user, you can run $IDF_PATH/export.sh and it will pick up the tools installed in /opt/esp. The only requirement is that IDF_TOOLS_PATH should be set to the same value in the user's environment.

Thanks, I would try if modifying the package to put IDF_TOOLS_PATH=/usr/share/esp-idf/tools in /etc/env.d fixes the issue

@stkw0
Copy link
Contributor Author

stkw0 commented Jul 11, 2022

Okay, so this doesn't work because obviously files installed by the package manager are not writable
IDF_TOOLS_PATH=/usr/share/esp-idf/tools idf -p /dev/ttyUSB0 build
gives

ERROR: File /usr/share/esp-idf/tools/idf-env.json is not accessible to write.

fatal: not a git repository: '/usr/share/esp-idf/.git'
WARNING: Git version unavailable, reading from source
ESP-IDF v5.0.0

@igrr
Copy link
Member

igrr commented Jul 11, 2022

I see, that's a bug which deserves to be fixed. We shouldn't require that IDF_TOOLS_PATH location is writable when running idf.py.

I'll open a new issue to track this fix.

@stkw0
Copy link
Contributor Author

stkw0 commented Jul 11, 2022

Okay, thank you for your time

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Aug 1, 2022
@HiFiPhile
Copy link
Contributor

HiFiPhile commented Aug 25, 2022

It's really useful to make locally reproducible workflow. I presume if I checked out a revision then all rebuild should be the same (except hash which has a date change) .

What happened actually is 2 weeks later after my vacation my build workflow failed, as it downloaded new espidf.constraints.v5.0.txt and complains esptool 4.2 should be installed instead of 4.1

And why this espidf.constraints.v5.0.txt file is downloaded separately from espressif server instead of revisioned in esp-idf ?

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Sep 19, 2022
espressif-bot pushed a commit that referenced this pull request Oct 10, 2022
… file

The Python dependency checker called from the export scripts and before
build remains offline, i.e. it will use the previously downloaded
constraint file but won't download a newer version.

Related to #9328
espressif-bot pushed a commit that referenced this pull request Oct 13, 2022
… file

The Python dependency checker called from the export scripts and before
build remains offline, i.e. it will use the previously downloaded
constraint file but won't download a newer version.

Related to #9328
espressif-bot pushed a commit that referenced this pull request Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants