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 2022/01/04 16:41:46 UTC

[GitHub] [incubator-nuttx] AlanRosenthal opened a new pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

AlanRosenthal opened a new pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164


   ## Summary
   With recent improvements to Makefiles in these PRs:
   * https://github.com/apache/incubator-nuttx/pull/5055
   * https://github.com/apache/incubator-nuttx/pull/5069
   * https://github.com/apache/incubator-nuttx/pull/5116
   * https://github.com/apache/incubator-nuttx/pull/5163 (not merged yet)
   * https://github.com/apache/incubator-nuttx/pull/5147 (not merged yet)
   
   The context rule now only needs to be run once.
   
   ## Impact
   We can verify that by running `make` (which will run `context` as part of the normal build).
   Then we can verify that by running `make context --question` which will return `0` if the target is up to date
   
   ## Testing
   Verified locally, once the final two PRs are merged CI should pass (and it should fail before that)
   


-- 
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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r778719017



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       I understand the purpose, but what I am saying is, are we going to generalize this kind of tests for the build system? It seems a bit out of scope of `testbuild.sh`.




-- 
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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r778542191



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       should we generate some log in the error case?




-- 
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



[GitHub] [incubator-nuttx] AlanRosenthal commented on pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#issuecomment-1010432587


   I've decided to approach this a bit different, see https://github.com/apache/incubator-nuttx/issues/5205


-- 
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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r778700904



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       I think that @AlanRosenthal just want to check "make context" doesn't do any action after the initial config, but it's hard to identify this issue if there is no any output to indicate the problem.




-- 
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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r778687313



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       What will be our plan here?  Should we add other targets as well?  Or create separate workflow for build system 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.

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

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



[GitHub] [incubator-nuttx] AlanRosenthal closed pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
AlanRosenthal closed pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164


   


-- 
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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r778746479



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       Why out of scope? 'make context' is also part of build process, I think.




-- 
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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5164: Add test to CI to verify that the `context` rule doesn't need to be rebuilt between runs

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5164:
URL: https://github.com/apache/incubator-nuttx/pull/5164#discussion_r779385033



##########
File path: tools/testbuild.sh
##########
@@ -258,6 +258,10 @@ function build {
     fail=1
   fi
 
+  # Verify that the context rule does not need to be rebuilt
+
+  makefunc context --question

Review comment:
       It is part of the build system, but in `testbuild.sh` we don't unit test the build system, it just builds a bunch of configurations.
   I don't have a strong feeling about this, but I wanted to know if we would want to add other parts of the build system 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.

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

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