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

fix: create-project & create_component with proper file permission (IDFGH-10599) #11836

Merged
merged 1 commit into from Jul 21, 2023

Conversation

ryan4yin
Copy link
Contributor

@ryan4yin ryan4yin commented Jul 8, 2023

On Linux distributions such as NixOS that emphasize immutability and reproducibility, the file permissions of all software packages are forced to be set to read-only to ensure that they will not be tampered with.
As a result, the following permission error occurs when using idf.py create-project on such systems, and this PR fix the problem:

› idf.py create-project ws2812_led_1
Executing action: create-project
Traceback (most recent call last):
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf.py", line 767, in <module>
    main()
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf.py", line 702, in main
    cli(sys.argv[1:], prog_name=PROG, complete_var=SHELL_COMPLETE_VAR)
  File "/nix/store/3rhkgqrzf4mza4c7h6ljh2iwnwgv77ca-python3-3.10.12-env/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/nix/store/3rhkgqrzf4mza4c7h6ljh2iwnwgv77ca-python3-3.10.12-env/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/nix/store/3rhkgqrzf4mza4c7h6ljh2iwnwgv77ca-python3-3.10.12-env/lib/python3.10/site-packages/click/core.py", line 1689, in invoke
    return _process_result(rv)
  File "/nix/store/3rhkgqrzf4mza4c7h6ljh2iwnwgv77ca-python3-3.10.12-env/lib/python3.10/site-packages/click/core.py", line 1626, in _process_result
    value = ctx.invoke(self._result_callback, value, **ctx.params)
  File "/nix/store/3rhkgqrzf4mza4c7h6ljh2iwnwgv77ca-python3-3.10.12-env/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf.py", line 605, in execute_tasks
    task(ctx, global_args, task.action_args)
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf.py", line 187, in __call__
    self.callback(self.name, context, global_args, **action_args)
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf_py_actions/create_ext.py", line 68, in create_new
    func_action_map[action](target_path, action_args['name'])
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf_py_actions/create_ext.py", line 46, in create_project
    replace_in_file(os.path.join(main_folder, 'CMakeLists.txt'), 'main', name)
  File "/nix/store/jc0qzi0322w9w1nsssbw1vbkhlkgm6p8-esp-idf-v5.1/tools/idf_py_actions/create_ext.py", line 20, in replace_in_file
    with open(filename, 'r+') as f:
PermissionError: [Errno 13] Permission denied: '/home/ryan/codes/esp/ws2812_led_1/main/CMakeLists.txt'

› ls -al $IDF_PATH/examples/get-started
total 8
dr-xr-xr-x 1 root root 120  1月  1  1970 .
dr-xr-xr-x 1 root root 346  1月  1  1970 ..
dr-xr-xr-x 1 root root  84  1月  1  1970 blink
-r--r--r-- 5 root root 158  1月  1  1970 .build-test-rules.yml
dr-xr-xr-x 1 root root 120  1月  1  1970 hello_world
-r--r--r-- 7 root root 197  1月  1  1970 README.md
dr-xr-xr-x 1 root root  54  1月  1  1970 sample_project

@CLAassistant
Copy link

CLAassistant commented Jul 8, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 8, 2023
@github-actions github-actions bot changed the title fix: create-project & create_component with proper file permission fix: create-project & create_component with proper file permission (IDFGH-10599) Jul 8, 2023
@igrr
Copy link
Member

igrr commented Jul 8, 2023

Thanks for the PR @ryan4yin! May I ask you to add a test case for this situation, similar to

def test_create_project(idf_py: IdfPyFunc, idf_copy: Path) -> None:
? You may use idf_copy fixture to create a copy of IDF and then make it read-only.

1 similar comment
@igrr
Copy link
Member

igrr commented Jul 8, 2023

Thanks for the PR @ryan4yin! May I ask you to add a test case for this situation, similar to

def test_create_project(idf_py: IdfPyFunc, idf_copy: Path) -> None:
? You may use idf_copy fixture to create a copy of IDF and then make it read-only.

@ryan4yin
Copy link
Contributor Author

ryan4yin commented Jul 9, 2023

@igrr Updated.

The new test function fails on the master branch:

xxx@19e608e16f68:/opt/esp/idf/tools/test_build_system$ pytest -k test_create_project_with_idf_readonly
=========================================================================================================== test session starts ============================================================================================================
platform linux -- Python 3.8.10, pytest-7.4.0, pluggy-1.2.0
rootdir: /opt/esp/idf/tools/test_build_system
configfile: pytest.ini
plugins: timeout-2.1.0, rerunfailures-12.0, embedded-1.3.3
collected 71 items / 70 deselected / 1 selected                                                                                                                                                                                            

test_common.py::test_create_project_with_idf_readonly 
-------------------------------------------------------------------------------------------------------------- live log call ---------------------------------------------------------------------------------------------------------------
2023-07-09 11:42:54 INFO Check that command for creating new project will success if the IDF itself is readonly.
FAILED                                                                                                                                                                                                                               [100%]

================================================================================================================= FAILURES =================================================================================================================
__________________________________________________________________________________________________ test_create_project_with_idf_readonly ___________________________________________________________________________________________________

idf_copy = PosixPath('/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf')

    def test_create_project_with_idf_readonly(idf_copy: Path) -> None:
        logging.info('Check that command for creating new project will success if the IDF itself is readonly.')
        def change_to_readonly(src):
            for root, dirs, files in os.walk(src):
                for name in dirs:
                    os.chmod(os.path.join(root, name), 0o555) # read & execute
                for name in files:
                    path = os.path.join(root, name)
                    if '/bin' in path: continue  # skip excutables
                    os.chmod(os.path.join(root, name), 0o444) # readonly
        change_to_readonly(idf_copy)
        ret = run_idf_py(
            'create-project', '--path', str(idf_copy / 'example_proj'), 'temp_test_project', check=False,
            env=get_idf_build_env(idf_copy)
        )
>       assert ret.returncode == 0, 'Command create-project exit value is wrong.'
E       AssertionError: Command create-project exit value is wrong.
E       assert 1 == 0
E        +  where 1 = CompletedProcess(args=['/opt/esp/python_env/idf5.2_py3.8_env/bin/python', '/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf.py', 'create-project', '--path', '/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/example_proj', 'temp_test_project'], returncode=1, stdout='Executing action: create-project\n', stderr='/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/check_python_dependencies.py:12: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html\n  import pkg_resources\nTraceback (most recent call last):\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf.py", line 781, in <module>\n    main()\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf.py", line 716, in main\n    cli(sys.argv[1:], prog_name=PROG, complete_var=SHELL_COMPLETE_VAR)\n  File "/opt/esp/python_env/idf5.2_py3.8_env/lib/python3.8/site-packages/click/core.py", line 1130, in __call__\n    return self.main(*args, **kwargs)\n  File "/opt/esp/python_env/idf5.2_py3.8_env/lib/python3.8/site-packages/click/core.py", line 1055, in main\n    rv = self.invoke(ctx)\n ... "/opt/esp/python_env/idf5.2_py3.8_env/lib/python3.8/site-packages/click/core.py", line 760, in invoke\n    return __callback(*args, **kwargs)\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf.py", line 613, in execute_tasks\n    task(ctx, global_args, task.action_args)\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf.py", line 185, in __call__\n    self.callback(self.name, context, global_args, **action_args)\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf_py_actions/create_ext.py", line 68, in create_new\n    func_action_map[action](target_path, action_args[\'name\'])\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf_py_actions/create_ext.py", line 46, in create_project\n    replace_in_file(os.path.join(main_folder, \'CMakeLists.txt\'), \'main\', name)\n  File "/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/tools/idf_py_actions/create_ext.py", line 20, in replace_in_file\n    with open(filename, \'r+\') as f:\nPermissionError: [Errno 13] Permission denied: \'/tmp/tmpjyi1lk3y/test_create_project_with_idf_readonly_idf/example_proj/main/CMakeLists.txt\'\n').returncode

test_common.py:255: AssertionError
------------------------------------------------------------------------------------------------------------ Captured log call -------------------------------------------------------------------------------------------------------------
INFO     root:test_common.py:241 Check that command for creating new project will success if the IDF itself is readonly.
========================================================================================================= short test summary info ==========================================================================================================
FAILED test_common.py::test_create_project_with_idf_readonly - AssertionError: Command create-project exit value is wrong.
===================================================================================================== 1 failed, 70 deselected in 2.21s =====================================================================================================

And passes on this PR(with preserve_mode=0):

xxx@19e608e16f68:/opt/esp/idf/tools/test_build_system$ pytest -k test_create_project_with_idf_readonly
=========================================================================================================== test session starts ============================================================================================================
platform linux -- Python 3.8.10, pytest-7.4.0, pluggy-1.2.0
rootdir: /opt/esp/idf/tools/test_build_system
configfile: pytest.ini
plugins: timeout-2.1.0, rerunfailures-12.0, embedded-1.3.3
collected 71 items / 70 deselected / 1 selected                                                                                                                                                                                            

test_common.py::test_create_project_with_idf_readonly 
-------------------------------------------------------------------------------------------------------------- live log call ---------------------------------------------------------------------------------------------------------------
2023-07-09 11:42:29 INFO Check that command for creating new project will success if the IDF itself is readonly.
PASSED                                                                                                                                                                                                                               [100%]

===================================================================================================== 1 passed, 70 deselected in 2.28s =====================================================================================================

@dobairoland
Copy link
Collaborator

Hi @ryan4yin. Thank you for your contribution. Could you please check the state of your pull request? I see only the new test case and it looks like the fix is missing. Thanks!

@ryan4yin
Copy link
Contributor Author

@dobairoland OK

@ryan4yin
Copy link
Contributor Author

@dobairoland @mfialaf Updated.

@dobairoland dobairoland requested a review from mfialaf July 11, 2023 10:09
Copy link
Collaborator

@mfialaf mfialaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryan4yin Thank you for this nice improvement! I have left a few notes.

Comment on lines 251 to 255
ret = run_idf_py(
'create-project', '--path', str(idf_copy / 'example_proj'), 'temp_test_project', check=False,
env=get_idf_build_env(idf_copy)
)
assert ret.returncode == 0, 'Command create-project exit value is wrong.'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for ret. code 0 is not necessary. If the function fails, the test fails.
Furthermore, since the env is automatically detected, there is no need to pass the default environment without any change.

Suggested change
ret = run_idf_py(
'create-project', '--path', str(idf_copy / 'example_proj'), 'temp_test_project', check=False,
env=get_idf_build_env(idf_copy)
)
assert ret.returncode == 0, 'Command create-project exit value is wrong.'
run_idf_py('create-project', '--path', str(idf_copy / 'example_proj'), 'temp_test_project')

@@ -13,7 +13,7 @@

import pytest
from test_build_system_helpers import (EnvDict, IdfPyFunc, append_to_file, find_python, get_snapshot, replace_in_file,
run_idf_py)
run_idf_py, get_idf_build_env)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then there is no need to import get_idf_build_env

Suggested change
run_idf_py, get_idf_build_env)
run_idf_py)

@@ -237,6 +237,24 @@ def test_create_project(idf_py: IdfPyFunc, idf_copy: Path) -> None:
assert ret.returncode == 4, 'Command create-project exit value is wrong.'


def test_create_project_with_idf_readonly(idf_copy: Path) -> None:
logging.info('Check that command for creating new project will success if the IDF itself is readonly.')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick. I would define the inner function first and then place the outer function commands.

def test_create_project_with_idf_readonly(idf_copy: Path) -> None:
    def change_to_readonly(src):
        ...

    logging.info('Check ... 

@ryan4yin
Copy link
Contributor Author

@mfialaf Thanks for the suggestion, updated~

@ryan4yin ryan4yin requested a review from mfialaf July 16, 2023 10:35
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jul 17, 2023
Copy link
Collaborator

@mfialaf mfialaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryan4yin Super work! Thank you.
Could you please rebase to the current master? That should fix the pipeline bug and then we can merge your PR.

@ryan4yin
Copy link
Contributor Author

@mfialaf Yep, Updated.

@mfialaf
Copy link
Collaborator

mfialaf commented Jul 17, 2023

sha=354e2c4673c407ece74f12f92ad3ef720720afcc

1 similar comment
@dobairoland
Copy link
Collaborator

sha=354e2c4673c407ece74f12f92ad3ef720720afcc

@dobairoland dobairoland added the PR-Sync-Merge Pull request sync as merge commit label Jul 17, 2023
@mfialaf
Copy link
Collaborator

mfialaf commented Jul 17, 2023

@mfialaf Yep, Updated.

Thank you. No need to fix the pre-commit issue. We will take care of that.

@espressif-bot espressif-bot added the Status: Reviewing Issue is being reviewed label Jul 18, 2023
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed labels Jul 18, 2023
@tom-borcin tom-borcin added Resolution: Done Issue is done internally and removed Resolution: NA Issue resolution is unavailable labels Jul 19, 2023
@espressif-bot espressif-bot merged commit 354e2c4 into espressif:master Jul 21, 2023
7 of 8 checks passed
@ryan4yin ryan4yin deleted the patch-1 branch March 9, 2024 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Sync-Merge Pull request sync as merge commit 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.

None yet

7 participants