xiaoxiang781216 commented on PR #14479: URL: https://github.com/apache/nuttx/pull/14479#issuecomment-2481789522
> > > > @xiaoxiang781216 i'm not sure which changes you are talking about. can you explain a bit? > > > > anyway, i guess the original PR should not have contained unrelated changes together. i suspect it's simpler to revert the PR as a whole and ask the author to re-submit those parts separately. > > > > > > > > > Ok, let's me revert. > > > > > > @yamt After comparing carefully all nuttx and littlefs patch, I think the change is good in general. The follow is my answer to your concern. > > > Reverts #13964 > > > because: > > > > > > * it seems to introduce some regressions. i saw fstat failing with ENOENT on files on an image generated with https://github.com/jrast/littlefs-python. > > > > > > Could you give a repro step? So, we can provide a fix. > > you can reproduce it with the instructions in https://nuttx.apache.org/docs/latest/platforms/xtensa/esp32s3/boards/esp32s3-devkit/index.html#toywasm. (with an update #14832) > @crafcat7 please find the root cause why the format fail with attribute patch. > > > * we should reduce the amount of the local patches. not the opposite. if you want to extend littlefs, please upstream it first. > > > > > > It upstream here: [littlefs-project/littlefs#1045](https://github.com/littlefs-project/littlefs/pull/1045). littlefs already provide lfs_setattr and lfs_getattr which work on file path, the new functions (lfs_file_getattr and lfs_file_setattr) which work on file handle is a natural extension. Let's wait the author feedback. > > > * it's controversial if it's a good idea to waste on-disk structures for attributes like file modes and uid/gid, which nuttx doesn't really support. > > > > > > uid/gid is useful after: #8924 #10119 #10176 [apache/nuttx-apps#1691](https://github.com/apache/nuttx-apps/pull/1691) This is the first real filesystem supported uid/gid/mode on NuttX, so it's better to keep it to demonstrate this capability. > > BTW, the attribute contains the date/time information which is very useful for most people. > > i don't object much if: > > * the patch is accepted by the upstream. (it's important for me to be able to use latest version of littlefs.) the patch already upstream, let's wait the feedback. > * and it's optional. (i don't want to use my flash space for these attributes.) Ok, @crafcat7 please add an option to skip the attribute operation. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
