You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/10 17:27:25 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request #358: Remove the unnecessary touch and delete from Makefile
xiaoxiang781216 opened a new pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358
## Summary
## Impact
No functionality change.
## Testing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468615229
##########
File path: testing/unity/Makefile
##########
@@ -68,7 +68,6 @@ $(UNITY_TARBALL):
$(UNITY_UNPACKNAME): $(UNITY_TARBALL)
@echo "Unpacking: $(UNITY_TARBALL) -> $(UNITY_UNPACKNAME)"
$(Q) $(UNPACK) $(UNITY_TARBALL)
- @touch $(UNITY_UNPACKNAME)
Review comment:
Ok, keep touch for unzip too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468169170
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
BTW, `tar` has the option --touch (or -m) that does updates the extracted folder's timestaps. But I believe explicitly calling `touch` is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468169170
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
BTW, `tar` has the option --touch (or -m) that does updates the extracted folder's timestamps. But I believe explicitly calling `touch` is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468504726
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
So it's your `mv` that's changing the timestamps?
It looks like it's an `mv` version issue. There are some explanations [here](https://unix.stackexchange.com/questions/503233/when-does-mv-change-the-timestamp-of-a-file-and-while-does-it-not).
I don't think we should rely on the details of `mv` implementation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468141280
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
@xiaoxiang781216 I think touch is still necessary here.
I was puzzled by this one, and thought it was unnecessary, please confirm the following:
1. We extract the folder, say LVGL-v2.2.0 (this folder will keep the timestamps when it was tar'ed)
2. We rename it as LVGL (it also keeps the timestamps of the above)
With that, the tarball is newer than the folder LVGL and the target will execute again with the next `make` and copy an extra LVGL-v2.2.0 inside LVGL. Now LVGL has changed and no more copies.
With `touch` we update the folder's timestamps and we won't have an extra copy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468169170
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
BTW, `tar` has the option --touch (or -m) that does make updates the extracted folder's timestaps. But I believe explicitly calling `touch` is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468322659
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
I tries two package(lvgl and libcanard), both don't hit the double downloading issue. @Ouss4 could you help me confirm?
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
I tries two package(lvgl and libcanard), both don't hit the double downloading issue. @Ouss4 could you help me confirm it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 merged pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468529964
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
Ok, I keep the touch immediately follow mv, but remove others. Please take a look, @Ouss4.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468399842
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
libcanard's Makefile deletes the folder before unpacking, so we can't see the issue there.
I just tried with lvgl, and I do get a second folder.
Is that an `mv` implementation difference? I'm running on Manjaro with Coretutils 8.32, what's your platform?
Could you please try to manually inspect the timestamps with ls -l?
> both don't hit the double downloading issue
It doesn't download the archive again, it just unpacks the existing tar or zip file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468533580
##########
File path: testing/unity/Makefile
##########
@@ -68,7 +68,6 @@ $(UNITY_TARBALL):
$(UNITY_UNPACKNAME): $(UNITY_TARBALL)
@echo "Unpacking: $(UNITY_TARBALL) -> $(UNITY_UNPACKNAME)"
$(Q) $(UNPACK) $(UNITY_TARBALL)
- @touch $(UNITY_UNPACKNAME)
Review comment:
This should stay too, unzip or tar will keep the timestamps of what's inside the archive.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468484798
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
Here is my test:
```
drwxr-xr-x 9 xiaoxiang xiaoxiang 4096 2020-08-11 17:35:48.659398700 +0800 lvgl
-rw-r--r-- 1 xiaoxiang xiaoxiang 6847211 2020-08-11 17:35:14.369398700 +0800 v7.0.2.zip
```
and mv --version:
```
xiaoxiang@DESKTOP-JH2VEP6:~/nuttx/nuttx$ mv --version
mv (GNU coreutils) 8.28
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Mike Parker, David MacKenzie, and Jim Meyering.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #358: Remove the unnecessary touch and delete from Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #358:
URL: https://github.com/apache/incubator-nuttx-apps/pull/358#discussion_r468169170
##########
File path: graphics/lvgl/Makefile
##########
@@ -74,13 +74,9 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
@echo "Unpacking: $(LVGL_TARBALL) -> $(LVGL_UNPACKNAME)"
$(Q) $(UNPACK) $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
- $(Q) touch $(LVGL_UNPACKNAME)
Review comment:
BTW, `tar` has the option --touch (or -m) that updates the extracted folder's timestamps. But I believe explicitly calling `touch` is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org