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 2021/02/26 06:34:45 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #2930: tools/version.h: If the version is not available, use 0.0.0
yamt opened a new pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930
## Summary
It's useful when you only have shallow git history handy.
(eg. to save some network bandwidth)
The default value here was chosen to mirror the default of
tools/Makefile.win.
## Impact
## Testing
build tested locally with a -depth=1 tree.
----------------------------------------------------------------
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] hartmannathan commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786722842
(My earlier comment was incorrect. I was thinking of a different change.)
Currently if no version information is available, the script returns an error code (1) and exits. This patch will change that to (silently, well almost) succeed and use the dummy value. I wonder if anyone is relying on the existing behavior of exit code, e.g., in a larger automated build system etc.
Suggestion:
Would it be better to introduce a new command line option that means "If version information cannot be found, use default (dummy) value"?
If the new command line option is used, we get the behavior of this PR, "use dummy value".
If not, we get the old behavior, "if version cannot be determined, exit with error code"
----------------------------------------------------------------
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] davids5 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786662662
> Just one comment, can we remove the similar code from Makefile.*:
> https://github.com/apache/incubator-nuttx/blob/master/tools/Makefile.unix#L24-L48
That breaks the existing tooling that was broken last time.
----------------------------------------------------------------
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] davids5 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786664732
Having a proper version from git is needed for projects.
I am fine with hard coded 0.0.0 in the absences of real version information, but not with removing the the real working one based one what is checked out.
----------------------------------------------------------------
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] Ouss4 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786666153
The similar part is just two lines from https://github.com/apache/incubator-nuttx/blob/master/tools/Makefile.unix#L34-L42. Where `.version` doesn't exist. I don't think @xiaoxiang781216 is suggesting to remove the whole release information retrieval from git.
----------------------------------------------------------------
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] hartmannathan commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-792182978
This PR has been waiting for quite a few days now. I'm okay with merging it so I'll go ahead and add my approval.
----------------------------------------------------------------
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] hartmannathan removed a comment on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
hartmannathan removed a comment on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786713990
> Just one comment, can we remove the similar code from Makefile.*:
> https://github.com/apache/incubator-nuttx/blob/master/tools/Makefile.unix#L24-L48
When users build from release artifacts (without .git directory) that would break the build for them.
----------------------------------------------------------------
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] hartmannathan commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786713990
> Just one comment, can we remove the similar code from Makefile.*:
> https://github.com/apache/incubator-nuttx/blob/master/tools/Makefile.unix#L24-L48
When users build from release artifacts (without .git directory) that would break the build for them.
----------------------------------------------------------------
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] xiaoxiang781216 merged pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930
----------------------------------------------------------------
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] davids5 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786668039
my bad then, if it just the default.
----------------------------------------------------------------
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] Ouss4 commented on pull request #2930: tools/version.h: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786485991
The Makefile already does this when no .git is present. The workaround for shallow cloning is to rename .git. But I do find this renaming a bit awkward.
----------------------------------------------------------------
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] Ouss4 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786641183
@xiaoxiang781216 any thing to consider here?
----------------------------------------------------------------
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] Ouss4 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-787500153
The thing with adding a new command line option is that we will have to control it somehow from the Makefile. Are we going to pass a variable with `make`? Or introduce a new CONFIG_? I think a lot of people will be against adding a Kconfig option just because we have too many.
Another idea that has been suggested before is to include the latest `.version` file, under a different name, with the repo and update it each release. Then `version.sh` will use that as a last resort.
----------------------------------------------------------------
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] hartmannathan commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-788018984
I don't think we should add a new CONFIG_ for this, because then people will have to remember to set it, or make a mistake and set it incorrectly.
> Another idea that has been suggested before is to include the latest `.version` file, under a different name, with the repo and update it each release. Then `version.sh` will use that as a last resort.
Instead of a file, put the value directly into `version.sh` instead of a dummy value? And forget my earlier idea of a new command option, but output a "warning: version cannot be found, using default value of x.x.x"?
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #2930: tools/version.sh: If the version is not available, use 0.0.0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2930:
URL: https://github.com/apache/incubator-nuttx/pull/2930#issuecomment-786661605
Just one comment, can we remove the similar code from Makefile.*:
https://github.com/apache/incubator-nuttx/blob/master/tools/Makefile.unix#L24-L48
----------------------------------------------------------------
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