You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "guohao15 (via GitHub)" <gi...@apache.org> on 2024/10/12 08:15:32 UTC

[PR] port inotify test for nuttx [nuttx-apps]

guohao15 opened a new pull request, #2702:
URL: https://github.com/apache/nuttx-apps/pull/2702

   ## Summary
   1 make the libuv run use epoll as backend
   2 make the inotify test case runable in nuttx
   
   ## Impact
   select epoll as backend may spend more stack memory 
   
   ## Testing
   
   uv_run_tests    fs_event_watch_dir
   uv_run_tests    fs_event_close_in_callback
   uv_run_tests    fs_event_watch_file
   uv_run_tests    fs_event_watch_file_current_dir
   uv_run_tests    fs_event_ref
   uv_run_tests    fs_event_immediate_close
   uv_run_tests    fs_event_watch_file_exact_path
   uv_run_tests    fs_event_no_callback_on_close
   uv_run_tests    fs_event_no_callback_after_close
   uv_run_tests    fs_event_close_with_pending_event
   uv_run_tests    fs_event_close_with_pending_delete_event
   uv_run_tests    fs_event_error_reporting
   uv_run_tests    fs_event_start_and_close
   uv_run_tests    fs_event_watch_file_twice  
   uv_run_tests    fs_event_getpath
   uv_run_tests    fs_event_watch_invalid_path   
   uv_run_tests    fs_event_stop_in_cb
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] port inotify test for nuttx [nuttx-apps]

Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #2702:
URL: https://github.com/apache/nuttx-apps/pull/2702#issuecomment-2408454916

   [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   The PR summary is lacking some details. Here's a breakdown and suggestions:
   
   **What's Missing in the Summary**
   
   * **Why:** The summary states WHAT is being changed ("make libuv use epoll", "make inotify test runnable"), but not WHY these changes are necessary. 
       * Is there a bug being fixed? 
       * Is this adding a new capability to NuttX? 
       * Is it improving performance?
   * **How (more details):**  The summary is very high-level. It would be helpful to elaborate on HOW these changes are implemented:
       * **libuv epoll:**  What parts of the libuv code were modified to support epoll? Were any new files added?
       * **inotify test:** What prevented the inotify test from running previously? What changes were needed to make it functional? 
   * **Issues/PRs:** The summary should explicitly link to any related NuttX issues or pull requests.
   
   **Impact Section: Needs More Detail**
   
   * **Stack Memory:** Quantify the impact. How much more stack memory is used? Is this a significant concern for resource-constrained devices?
   * **Other Impacts:** The PR focuses solely on stack memory. Consider these questions:
       * **Build:** Will any build system changes be needed (new configuration options, dependencies)?
       * **Hardware:**  Does epoll have any specific hardware requirements or dependencies?
       * **Documentation:**  Will documentation need to be updated to reflect these changes?
       * **Compatibility:**  Could these changes potentially break existing applications or have compatibility implications?
   
   **Testing Section:  Missing Context**
   
   * **Testing Environment:** Provide details about your testing setup:
       * Which NuttX version are you using?
       * What is your target hardware (or simulator)?
       * Which compiler and toolchain were used?
   * **Log Snippets:** The current logs are just test names. Include relevant snippets of the logs that demonstrate the changes in behavior before and after your modifications. Focus on:
       * **libuv epoll:**  Show logs that confirm epoll is being used correctly. 
       * **inotify test:** Show that the test, which previously failed, now passes.
   
   **In conclusion:**  The PR needs more detail and context to meet NuttX requirements. Focus on explaining the "why" behind the changes, providing specifics about the implementation, quantifying the impact, and including relevant testing details. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] port inotify test for nuttx [nuttx-apps]

Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #2702:
URL: https://github.com/apache/nuttx-apps/pull/2702#issuecomment-2409260342

   Looks like build errors to be fixed? Will restart failed CI tasks :-)
   
   ```
   (...)
   x luv-1.44.2-1/src/fs_poll.c
   x luv-1.44.2-1/src/thread.c
   x luv-1.44.2-1/src/lhandle.c
   x luv-1.44.2-1/README.md
   
   100  2058    0  2058    0     0   2955      0 --:--:-- --:--:-- --:--:--  2955
   100 1646k    0 1646k    0     0  1454k      0 --:--:--  0:00:01 --:--:-- 3779k
   Error: libuv/src/threadpool.c:201:5: error: use of undeclared identifier 'CONFIG_LIBUV_THREADPOOL_PRIORITY'
       DEF_THREADPOOL_PRIORITY
       ^
   <command line>:14:33: note: expanded from macro 'DEF_THREADPOOL_PRIORITY'
   #define DEF_THREADPOOL_PRIORITY CONFIG_LIBUV_THREADPOOL_PRIORITY
                                   ^
   1 error generated.
   make[2]: *** [threadpool.c.Users.runner.work.nuttx-apps.nuttx-apps.sources.apps.system.libuv.o] Error 1
   make[2]: Target `all' not remade because of errors.
   make[1]: *** [/Users/runner/work/nuttx-apps/nuttx-apps/sources/apps/system/libuv_all] Error 2
   make[1]: Target `all' not remade because of errors.
   make: *** [/Users/runner/work/nuttx-apps/nuttx-apps/sources/apps/libapps.a] Error 2
   make: Target `all' not remade because of errors.
   /Users/runner/work/nuttx-apps/nuttx-apps/sources/nuttx/tools/testbuild.sh: line 385: /Users/runner/work/nuttx-apps/nuttx-apps/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
     Normalize sim/lua
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] port inotify test for nuttx [nuttx-apps]

Posted by "GUIDINGLI (via GitHub)" <gi...@apache.org>.
GUIDINGLI merged PR #2702:
URL: https://github.com/apache/nuttx-apps/pull/2702


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org