[PATCH v1] fix compiling warning
From: Zhongjie Zhu there are different cases for _ISR_lock_ISR_disable() and _ISR_lock_ISR_enable() in the case RTEMS_SMP is defined or RTEMS_PROFILING is defined, so remove the related code. ../../../cpukit/include/rtems/score/threaddispatch.h: In function '_Thread_Dispatch_disable': ../../../cpukit/include/rtems/score/threaddispatch.h:231:14: warning: 'lock_context' may be used uninitialized [-Wmaybe-uninitialized] 231 | cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); | ^~ ../../../cpukit/include/rtems/score/threaddispatch.h:210:32: note: by argument 1 of type 'const ISR_lock_Context *' to '_Thread_Dispatch_disable_critical' declared here 210 | static inline Per_CPU_Control *_Thread_Dispatch_disable_critical( |^ ../../../cpukit/include/rtems/score/threaddispatch.h:225:21: note: 'lock_context' declared here 225 | ISR_lock_Context lock_context; | ^~~~ --- cpukit/include/rtems/score/threaddispatch.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpukit/include/rtems/score/threaddispatch.h b/cpukit/include/rtems/score/threaddispatch.h index 589935823f..b06ebe8fec 100644 --- a/cpukit/include/rtems/score/threaddispatch.h +++ b/cpukit/include/rtems/score/threaddispatch.h @@ -222,16 +222,17 @@ static inline Per_CPU_Control *_Thread_Dispatch_disable_critical( static inline Per_CPU_Control *_Thread_Dispatch_disable( void ) { Per_CPU_Control *cpu_self; - ISR_lock_Context lock_context; #if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) + ISR_lock_Context lock_context; + _ISR_lock_ISR_disable( &lock_context ); -#endif cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); -#if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) _ISR_lock_ISR_enable( &lock_context ); +#else + cpu_self = _Thread_Dispatch_disable_critical( NULL ); #endif return cpu_self; -- 2.34.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1] fix compiling warning
On 16.01.23 09:36, Zhu Zhongjie wrote: From: Zhongjie Zhu there are different cases for _ISR_lock_ISR_disable() and _ISR_lock_ISR_enable() in the case RTEMS_SMP is defined or RTEMS_PROFILING is defined, so remove the related code. ../../../cpukit/include/rtems/score/threaddispatch.h: In function '_Thread_Dispatch_disable': ../../../cpukit/include/rtems/score/threaddispatch.h:231:14: warning: 'lock_context' may be used uninitialized [-Wmaybe-uninitialized] 231 | cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); | ^~ ../../../cpukit/include/rtems/score/threaddispatch.h:210:32: note: by argument 1 of type 'const ISR_lock_Context *' to '_Thread_Dispatch_disable_critical' declared here 210 | static inline Per_CPU_Control *_Thread_Dispatch_disable_critical( |^ ../../../cpukit/include/rtems/score/threaddispatch.h:225:21: note: 'lock_context' declared here 225 | ISR_lock_Context lock_context; | ^~~~ --- cpukit/include/rtems/score/threaddispatch.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpukit/include/rtems/score/threaddispatch.h b/cpukit/include/rtems/score/threaddispatch.h index 589935823f..b06ebe8fec 100644 --- a/cpukit/include/rtems/score/threaddispatch.h +++ b/cpukit/include/rtems/score/threaddispatch.h @@ -222,16 +222,17 @@ static inline Per_CPU_Control *_Thread_Dispatch_disable_critical( static inline Per_CPU_Control *_Thread_Dispatch_disable( void ) { Per_CPU_Control *cpu_self; - ISR_lock_Context lock_context; #if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) + ISR_lock_Context lock_context; + _ISR_lock_ISR_disable( &lock_context ); -#endif cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); -#if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) _ISR_lock_ISR_enable( &lock_context ); +#else + cpu_self = _Thread_Dispatch_disable_critical( NULL ); #endif return cpu_self; I doubt that this patch compiles if RTEMS_SMP or RTEMS_PROFILING is not defined. The problem with the warning is that _Thread_Dispatch_disable_critical() takes a const pointer to lock_context and nobody writes to this structure before it is handed over. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1] fix compiling warning
with RTEMS_SMP or RTEMS_PROFILING defined, it will call _Thread_Dispatch_disable_critical( &lock_context ) only in UP system with RTEMS_PROFILING not defined, it will call _Thread_Dispatch_disable_critical( NULL ), in this case, the real function use the lock_context is _Profiling_Thread_dispatch_disable_critical(), it is an empty function. I have tried the three cases. it could also be avoid by initialize the lock_context = {}; Sebastian Huber 于2023年1月16日周一 17:02写道: > > On 16.01.23 09:36, Zhu Zhongjie wrote: > > From: Zhongjie Zhu > > > > there are different cases for _ISR_lock_ISR_disable() and > > _ISR_lock_ISR_enable() > > in the case RTEMS_SMP is defined or RTEMS_PROFILING is defined, so remove > > the related code. > > > > ../../../cpukit/include/rtems/score/threaddispatch.h: In function > > '_Thread_Dispatch_disable': > > ../../../cpukit/include/rtems/score/threaddispatch.h:231:14: warning: > > 'lock_context' may be used uninitialized [-Wmaybe-uninitialized] > >231 | cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); > >| ^~ > > ../../../cpukit/include/rtems/score/threaddispatch.h:210:32: note: by > > argument 1 of type 'const ISR_lock_Context *' to > > '_Thread_Dispatch_disable_critical' declared here > >210 | static inline Per_CPU_Control *_Thread_Dispatch_disable_critical( > >|^ > > ../../../cpukit/include/rtems/score/threaddispatch.h:225:21: note: > > 'lock_context' declared here > >225 | ISR_lock_Context lock_context; > >| ^~~~ > > --- > > cpukit/include/rtems/score/threaddispatch.h | 7 --- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/cpukit/include/rtems/score/threaddispatch.h > > b/cpukit/include/rtems/score/threaddispatch.h > > index 589935823f..b06ebe8fec 100644 > > --- a/cpukit/include/rtems/score/threaddispatch.h > > +++ b/cpukit/include/rtems/score/threaddispatch.h > > @@ -222,16 +222,17 @@ static inline Per_CPU_Control > > *_Thread_Dispatch_disable_critical( > > static inline Per_CPU_Control *_Thread_Dispatch_disable( void ) > > { > > Per_CPU_Control *cpu_self; > > - ISR_lock_Context lock_context; > > > > #if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) > > + ISR_lock_Context lock_context; > > + > > _ISR_lock_ISR_disable( &lock_context ); > > -#endif > > > > cpu_self = _Thread_Dispatch_disable_critical( &lock_context ); > > > > -#if defined( RTEMS_SMP ) || defined( RTEMS_PROFILING ) > > _ISR_lock_ISR_enable( &lock_context ); > > +#else > > + cpu_self = _Thread_Dispatch_disable_critical( NULL ); > > #endif > > > > return cpu_self; > > I doubt that this patch compiles if RTEMS_SMP or RTEMS_PROFILING is not > defined. > > The problem with the warning is that _Thread_Dispatch_disable_critical() > takes a const pointer to lock_context and nobody writes to this > structure before it is handed over. > > -- > embedded brains GmbH > Herr Sebastian HUBER > Dornierstr. 4 > 82178 Puchheim > Germany > email: sebastian.hu...@embedded-brains.de > phone: +49-89-18 94 741 - 16 > fax: +49-89-18 94 741 - 08 > > Registergericht: Amtsgericht München > Registernummer: HRB 157899 > Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler > Unsere Datenschutzerklärung finden Sie hier: > https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCHES rtems, source-builder] Add GitHub Actions scripts
Hello, some weeks ago I created a GitHub Actions based CI script that we (embedded brains) wanted to use to test patches (see https://github.com/embedded-brains/rtems/tree/ci). I don't think much of the RTEMS community noted these. I would like to suggest adding the scripts to the official RTEMS repositories so that the actions are executed in the official GitHub RTEMS mirrors. To make sure that GitHub pull requests are not perceived at the official way to make RTEMS contributions, an auto-responder action notifies the pull request user that the current way to make contributions is sending patch sets to devel@rtems.org. This step will allow users to easily test patches on a number of simulators before they send them to the mailing list. No one is forced to do it, but everyone can try it. For RTEMS, it has the advantage that the patches are at least guaranteed to be compile-clean on a selected number of BSPs and that they survived a test run on a simulator. Please note: With this patch I do not intent to push GitHub as the RTEMS CI or to move from mailing list patches to push-requests. My idea is to allow everyone to experiment with a proof of concept prototype. Based on your experiences in this test phase, I would suggest that we have a review discussion in a month or two to select a suitable way forward for RTEMS CI. I think after that test phase we all know better what we want or expect which helps selecting the best CI system that then can replace this proof of concept system with GitHub. But now to make make it more clear what we will get with merging these patches: You can find a (not yet cleaned up) version of the patches in these repositories: https://github.com/embedded-brains/rtems https://github.com/embedded-brains/rtems-source-builder The results from a current run on RTEMS are here: https://github.com/embedded-brains/rtems/actions/runs/3901364934 If you scroll down on that page, you get a summary that shows which tests fail on three (mostly) randomly selected simulator BSPs. GR740 usually can run all tests but currently jffs2_fsrdwr fails. The full output of the rtems tester is in the Artifacts in case you want to take a look at the test output. If you want to try the CI with some of your patches before we merge this to the official repositories, feel free to create a pull request to the ci branches of the embedded-brains/rtems repositories. See the github manual for guidance how to create a pull request: https://docs.github.com/articles/creating-a-pull-request Note: It is important that you somewhen forked from the official RTEMS repositories or from one of the forks using (for example) the fork button in the github web interface. If you just pushed a repo to an empty one, github doesn't recognize the link and won't allow you to create a pull-request towards the embedded-brains repository. Best regards Christian ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH rtems 2/2] github: Mark pull requests as stale
Mark all new pull requests as stale. Add a note that the CI can be used but that patches should be sent to the mailing list instead. Close pull requests after 30 days. --- .github/workflows/mark-stale.yml | 23 +++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/mark-stale.yml diff --git a/.github/workflows/mark-stale.yml b/.github/workflows/mark-stale.yml new file mode 100644 index 00..9fd22d240b --- /dev/null +++ b/.github/workflows/mark-stale.yml @@ -0,0 +1,23 @@ +name: 'Mark all PRs as stale' +on: + pull_request: +types: [opened] + schedule: +- cron: '28 6 * * *' + workflow_dispatch: + +permissions: + issues: write + pull-requests: write + +jobs: + stale: +runs-on: ubuntu-latest +steps: + - uses: actions/stale@v7 +with: + stale-pr-message: 'Please note that Pull Requests are neither merged nor reviewed! Your Pull Requests only trigger continuous integration builds and tests in this repository and will automatically be closed after a month. As soon as you are happy with the result, please send the patches to the mailing list at devel@rtems.org. See https://lists.rtems.org for instructions how to register for that list. Thank you. ' + days-before-stale: -1 + days-before-close: -1 + days-before-pr-stale: 0 + days-before-pr-close: 30 -- 2.35.3 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH rtems-source-builder 1/2] github: Enable CI
Allow users to optionally use the GitHub CI. --- .github/workflows/toolchain.yml | 196 1 file changed, 196 insertions(+) create mode 100644 .github/workflows/toolchain.yml diff --git a/.github/workflows/toolchain.yml b/.github/workflows/toolchain.yml new file mode 100644 index 000..f9fec95 --- /dev/null +++ b/.github/workflows/toolchain.yml @@ -0,0 +1,196 @@ +name: Toolchain + +on: + # Triggers the workflow on push or pull request for the given branch + push: +branches: [ master ] + pull_request: +branches: [ master ] + + # Trigger manually from the Actions tab + workflow_dispatch: + +jobs: + devel: +strategy: + fail-fast: false + matrix: +tool: + - autotools-base + - autotools + - capstone + - dtc + - gnu-default-tools + - libtool + - libusb + - or1ksim + - qemu-couverture + - qemu-xilinx + - qemu + - sis + - spike + - swig +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v3 +with: + path: rsb + - name: Install dependencies +run: | + sudo apt-get update + sudo apt-get install \ +build-essential \ +ninja-build \ +libudev-dev + - name: Build ${{ matrix.tool }} +run: | + cd rsb/rtems + ../source-builder/sb-set-builder \ +--log=l-${{ matrix.tool }} \ +--prefix=/opt/rtems/6 \ +--bset-tar-file \ +devel/${{ matrix.tool }} + - name: Archive log on failure +if: ${{ failure() }} +uses: actions/upload-artifact@v3 +with: + name: log-${{ matrix.tool }} + path: | +rsb/rtems/l-${{ matrix.tool }} +rsb/**/rsb-report-*.txt + - name: Archive artifacts +uses: actions/upload-artifact@v3 +with: + name: devel-${{ matrix.tool }} + path: rsb/rtems/tar/*.tar* + + build: +strategy: + fail-fast: false + matrix: +target: + - aarch64 + - arm + - bfin + - i386 + - lm32 + - m68k + - microblaze + - mips + - moxie + - nios2 + - or1k + - powerpc + - riscv + - sh + - sparc + - sparc64 + - v850 + - x86_64 +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v3 +with: + path: rsb + - uses: actions/checkout@v3 +with: + path: rtems + ref: ci + repository: RTEMS/rtems + - name: Install dependencies +run: | + sudo apt-get update + sudo apt-get install \ +build-essential \ +flex \ +bison \ +cmake \ +texinfo \ +device-tree-compiler \ +u-boot-tools \ +lzop \ +libusb-1.0-0-dev \ +python3 \ +python-is-python3 \ +libpython3-dev \ +python3-dev + - name: Build toolchain +run: | + cd rsb/rtems + ../source-builder/sb-set-builder \ +--log=l-${{ matrix.target }} \ +--prefix=/opt/rtems/6 \ +--bset-tar-file \ +6/rtems-${{ matrix.target }} + - name: Archive log on failure +if: ${{ failure() }} +uses: actions/upload-artifact@v3 +with: + name: log-${{ matrix.target }} + path: rsb/rtems/l-${{ matrix.target }} + - name: Archive artifacts +uses: actions/upload-artifact@v3 +with: + name: tools-${{ matrix.target }} + path: rsb/rtems/tar/*.tar* + #- name: Build BSPs + # uses: ./rtems/.github/actions/build-bsps + # with: + #target: ${{ matrix.target }} + #sources-rtems: "${GITHUB_WORKSPACE}/rtems" + #prefix: /opt/rtems/6 + + simulator: +# run even if not all of the earlier matrix builds worked +if: ${{ always() }} +needs: + - build + - devel +strategy: + fail-fast: false + matrix: +simulators: + - sparc/gr740 + - riscv/griscv + - arm/xilinx_zynq_a9_qemu +runs-on: ubuntu-latest +steps: + - name: split arch and BSP +shell: bash +run: | + arch=`echo "${{ matrix.simulators }}" | sed -e "s|/.*||g"` + bsp=`echo "${{ matrix.simulators }}" | sed -e "s|.*/||g"` + echo "arch=${arch}" >> $GITHUB_ENV + echo "bsp=${bsp}" >> $GITHUB_ENV + - uses: actions/checkout@v3 +with: + path: rtems + ref: ci + repository: RTEMS/rtems + - name: Install dependencies +run: | + sudo apt-get update + sudo apt-get install \ +python3 \ +pyth
[PATCH rtems-source-builder 2/2] github: Mark pull requests as stale
Mark all new pull requests as stale. Add a note that the CI can be used but that patches should be sent to the mailing list instead. Close pull requests after 30 days. --- .github/workflows/mark-stale.yml | 23 +++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/mark-stale.yml diff --git a/.github/workflows/mark-stale.yml b/.github/workflows/mark-stale.yml new file mode 100644 index 000..9fd22d2 --- /dev/null +++ b/.github/workflows/mark-stale.yml @@ -0,0 +1,23 @@ +name: 'Mark all PRs as stale' +on: + pull_request: +types: [opened] + schedule: +- cron: '28 6 * * *' + workflow_dispatch: + +permissions: + issues: write + pull-requests: write + +jobs: + stale: +runs-on: ubuntu-latest +steps: + - uses: actions/stale@v7 +with: + stale-pr-message: 'Please note that Pull Requests are neither merged nor reviewed! Your Pull Requests only trigger continuous integration builds and tests in this repository and will automatically be closed after a month. As soon as you are happy with the result, please send the patches to the mailing list at devel@rtems.org. See https://lists.rtems.org for instructions how to register for that list. Thank you. ' + days-before-stale: -1 + days-before-close: -1 + days-before-pr-stale: 0 + days-before-pr-close: 30 -- 2.35.3 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH rtems 1/2] github: Enable CI
Allow users to optionally use the CI from github. --- .github/actions/build-bsps/action.yml| 49 .github/actions/run-simulator/action.yml | 144 +++ .github/workflows/bsps.yml | 63 ++ .github/workflows/simulator.yml | 72 4 files changed, 328 insertions(+) create mode 100644 .github/actions/build-bsps/action.yml create mode 100644 .github/actions/run-simulator/action.yml create mode 100644 .github/workflows/bsps.yml create mode 100644 .github/workflows/simulator.yml diff --git a/.github/actions/build-bsps/action.yml b/.github/actions/build-bsps/action.yml new file mode 100644 index 00..c06e560125 --- /dev/null +++ b/.github/actions/build-bsps/action.yml @@ -0,0 +1,49 @@ +name: "Build BSPs" +description: "Build BSPs for the given target architecture." + +inputs: + bsp: +description: "The bsp to build 'sparc/erc32'" +type: string +required: true + sources-rtems: +description: "Where to find the RTEMS sources. Use absolute path!" +type: string +required: true + prefix: +description: "Prefix directory with the installed tools. Use absolute path!" +type: string +required: true + +runs: + using: "composite" + steps: +- name: create name for logs + shell: bash + run: | +shortname=`echo "${{ inputs.bsp }}" | sed -e "s|/|_|g"` +echo "shortname=${shortname}" >> $GITHUB_ENV +- name: create log dir + shell: bash + run: mkdir "${GITHUB_WORKSPACE}"/bsp-logs +- name: Run BSP builder + shell: bash + run: | +"${{ inputs.prefix }}"/bin/rtems-bsp-builder \ + --build-path "${GITHUB_WORKSPACE}"/rtems-build \ + --prefix "${{ inputs.prefix }}" \ + --rtems-tools "${{ inputs.prefix }}/bin" \ + --rtems "${{ inputs.sources-rtems }}" \ + --bsp ${{ inputs.bsp }} \ + --log "${GITHUB_WORKSPACE}"/bsp-logs/log-${{ env.shortname }}.txt \ + --warnings-report "${GITHUB_WORKSPACE}"/bsp-logs/warnings-${{ env.shortname }}.txt \ + --failures-report "${GITHUB_WORKSPACE}"/bsp-logs/failures-${{ env.shortname }}.txt +- name: Archive log + uses: actions/upload-artifact@v3 + with: +name: log-${{ env.shortname }} +path: bsp-logs +- name: Check for errors + shell: bash + run: | +grep "Failures: 0" "$GITHUB_WORKSPACE"/bsp-logs/log-${{ env.shortname }}.txt diff --git a/.github/actions/run-simulator/action.yml b/.github/actions/run-simulator/action.yml new file mode 100644 index 00..23aa70e8ea --- /dev/null +++ b/.github/actions/run-simulator/action.yml @@ -0,0 +1,144 @@ +name: "Run Simulator" +description: "Build BSPs for the given target/BSP and run tests on a simulator" + +inputs: + target: +description: "The Arch/BSP to run. For example 'sparc/erc32'" +type: string +required: true + sources-rtems: +description: "Where to find the RTEMS sources. Use absolute path!" +type: string +required: true + prefix: +description: "Prefix directory with the installed tools. Use absolute path!" +type: string +required: true + +runs: + using: "composite" + steps: +- shell: bash + run: | +# Find correct parameters for this simulator +args="" +found=0 +[ "${{ inputs.target }}" == "sparc/erc32" ] && \ + args="--rtems-bsp=erc32-sis" && \ + found=1 +[ "${{ inputs.target }}" == "sparc/gr740" ] && \ + args="--rtems-bsp=gr740-sis" && \ + found=1 +[ "${{ inputs.target }}" == "riscv/griscv" ] && \ + args="--rtems-bsp=griscv-sis" && \ + found=1 +[ "${{ inputs.target }}" == "arm/xilinx_zynq_a9_qemu" ] && \ + args="--rtems-bsp=xilinx_zynq_a9_qemu" && \ + found=1 +echo "rtems_test_args=${args}" >> $GITHUB_ENV +echo "simulator_supported=${found}" >> $GITHUB_ENV +- if: ${{ env.simulator_supported == 0 }} + shell: bash + run: | +# Check whether parameters for this simulator have been found +echo "${{ inputs.target }} is not supported for simulator runs" +false +- shell: bash + run: | +# create environment variables for paths and names +shortname=`echo "${{ inputs.target }}" | sed -e "s|/|_|g"` +echo "shortname=${shortname}" >> $GITHUB_ENV +echo "logdir=${GITHUB_WORKSPACE}/logs-${shortname}" >> $GITHUB_ENV +echo "builddir=${GITHUB_WORKSPACE}/build-${shortname}" >> $GITHUB_ENV +- shell: bash + run: | +# create directories +mkdir -p "${{ env.logdir }}" +mkdir -p "${{ env.builddir }}" +- shell: bash + run: | +# generate BSP config +echo -e "[${{ inputs.tar
Re: [PATCH v1] fix compiling warning
On 16.01.23 10:38, 朱忠杰 wrote: with RTEMS_SMP or RTEMS_PROFILING defined, it will call _Thread_Dispatch_disable_critical( &lock_context ) only in UP system with RTEMS_PROFILING not defined, it will call _Thread_Dispatch_disable_critical( NULL ), in this case, the real function use the lock_context is _Profiling_Thread_dispatch_disable_critical(), it is an empty function. I have tried the three cases. it could also be avoid by initialize the lock_context = {}; Sorry, I misread your patch. I think it is doing the right thing. I will test it tomorrow. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/1] RSB: Mitigate too short error reports
Hi Chris, On 1/16/23 01:02, Chris Johns wrote: Subject: Re: [PATCH 1/1] RSB: Mitigate too short error reports From: Chris Johns Date: 1/16/23, 01:02 To: Frank Kühndel , devel@rtems.org On 22/12/2022 9:09 pm, Frank Kühndel wrote: On 12/21/22 00:06, Chris Johns wrote: On 21/12/2022 3:44 am, Frank Kuehndel wrote: From: Frank Kühndel Close #4642 --- source-builder/sb/ereport.py | 4 1 file changed, 4 insertions(+) diff --git a/source-builder/sb/ereport.py b/source-builder/sb/ereport.py index d8fb5f6..d391917 100755 --- a/source-builder/sb/ereport.py +++ b/source-builder/sb/ereport.py @@ -55,6 +55,10 @@ def generate(name, opts, header = None, footer = None): with open(name, 'w') as l: l.write(os.linesep.join(r)) log.notice(' See error report: %s' % (name)) + log.notice(' (Hint: The first error may be in front of a ' + 'line containing\n' + ' "Error 1" [GNU make] and may be only in the whole log ' Is this too specific to GNU make? What ifs a package uses cmake or something else? As the text indicates, this is specific to GNU make. For those using something else reading this text will still hint that the first error may not be in the end of the report "and may be only in the whole log". Another weak point in this text is that by far not all errors originating from "make". Yet, the true trouble is the original "See error report: %s" where it can sometimes happen that the error is not in this "error report" at all. I found it difficult to find a wording which is short, clear and helpful to the reader and at the same time not confusing. I am not perfectly happy with the notice above. I just found it a reasonable compromise. If you prefer more generic texts - such as the examples below - I will send a new patch with the suggested test. "Hint: The first error may be far way from the end of the report and in cases can only be found in the whole build log." "Hint: The error is most likely in the error report otherwise see the whole build log [--log option]." If you find any such change might cause more confusion than it might be helpful, I think it better to close this bug than to try to fix it. I think all you have written is valid and I have found the wording difficult. There will never be a robust error message scanner or a simple full proof way to find errors. The parallel builds makes tracking the errors difficult and the point of error and end of the build a long distance apart. As a result I question the value of the report and wonder if it should be removed. The report adds overhead to the build as the logging process needs to maintain a buffer of lines that is always updating. Your attention and interest around this feature highlights how problematic it is so maybe it is simpler and better to remove it and we leave users to find the error in the log file. I am happy to accept the report has not worked as a feature, remove it and in the process we recover some overheads in the logging area of the RSB? I am not against the error report and I do not say it is a useless feature. It is just that I found the message ' See error report: %s' confusing in those cases where the report does not contain the error at all because it is too short (the error report consists simply of the last 400 lines of the build log). To answer your question, I believe there is always a build log - no matter whether the `--log` option is used or not. In this case, removing the error report and pointing to the build log in case of error (for example like ' See build log: %s') would certainly solve all my concerns. On the other hand, implementing the error report took time and was certainly done with good reason. I do not feel like I should be the one deciding to remove it. Changing the message or simply closing https://devel.rtems.org/ticket/4642 would also be perfectly valid for me. Greetings ... and a happy new year to you Frank -- embedded brains GmbH Herr Frank KÜHNDEL Dornierstr. 4 82178 Puchheim Germany email: frank.kuehn...@embedded-brains.de phone: +49-89-18 94 741 - 23 mobile: +49-176-15 22 06 - 11 fax:+49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/1] RSB: Mitigate too short error reports
On Mon, Jan 16, 2023 at 8:46 AM Frank Kühndel < frank.kuehn...@embedded-brains.de> wrote: > Hi Chris, > > On 1/16/23 01:02, Chris Johns wrote: > > Subject: > > Re: [PATCH 1/1] RSB: Mitigate too short error reports > > From: > > Chris Johns > > Date: > > 1/16/23, 01:02 > > > > To: > > Frank Kühndel , devel@rtems.org > > > > > > On 22/12/2022 9:09 pm, Frank Kühndel wrote: > >> On 12/21/22 00:06, Chris Johns wrote: > >>> On 21/12/2022 3:44 am, Frank Kuehndel wrote: > From: Frank Kühndel > > Close #4642 > --- > source-builder/sb/ereport.py | 4 > 1 file changed, 4 insertions(+) > > diff --git a/source-builder/sb/ereport.py > b/source-builder/sb/ereport.py > index d8fb5f6..d391917 100755 > --- a/source-builder/sb/ereport.py > +++ b/source-builder/sb/ereport.py > @@ -55,6 +55,10 @@ def generate(name, opts, header = None, footer = > None): > with open(name, 'w') as l: > l.write(os.linesep.join(r)) > log.notice(' See error report: %s' % (name)) > +log.notice(' (Hint: The first error may be in front of > a ' > +'line containing\n' > +' "Error 1" [GNU make] and may be only in the whole > log ' > >>> Is this too specific to GNU make? What ifs a package uses cmake or > something > >>> else? > >> As the text indicates, this is specific to GNU make. For those using > something > >> else reading this text will still hint that the first error may not be > in the > >> end of the report "and may be only in the whole log". > >> > >> Another weak point in this text is that by far not all errors > originating from > >> "make". Yet, the true trouble is the original "See error report: %s" > where it > >> can sometimes happen that the error is not in this "error report" at > all. > >> > >> I found it difficult to find a wording which is short, clear and > helpful to the > >> reader and at the same time not confusing. I am not perfectly happy > with the > >> notice above. I just found it a reasonable compromise. > >> > >> If you prefer more generic texts - such as the examples below - I will > send a > >> new patch with the suggested test. > >> > >> "Hint: The first error may be far way from the end of the > >> report and in cases can only be found in the whole build log." > >> > >> "Hint: The error is most likely in the error report otherwise > >> see the whole build log [--log option]." > >> > >> If you find any such change might cause more confusion than it might be > helpful, > >> I think it better to close this bug than to try to fix it. > >> > > I think all you have written is valid and I have found the wording > difficult. > > There will never be a robust error message scanner or a simple full > proof way to > > find errors. The parallel builds makes tracking the errors difficult and > the > > point of error and end of the build a long distance apart. > I usually search the logs for "rror:" and that's the first time something is reported whether by make or gcc or whatever. It may not be the root cause but it gets me to the first report. Cutting any of these long reports down is always going to be possible to cut out the real issue. It's ok because it it's more than just an odd setup issue on the host, someone will have to build locally to reproduce the issue. And then they will get the full output. > > > > As a result I question the value of the report and wonder if it should be > > removed. The report adds overhead to the build as the logging process > needs to > > maintain a buffer of lines that is always updating. Your attention and > interest > > around this feature highlights how problematic it is so maybe it is > simpler and > > better to remove it and we leave users to find the error in the log file. > > > > I am happy to accept the report has not worked as a feature, remove it > and in > > the process we recover some overheads in the logging area of the RSB? > > > > I am not against the error report and I do not say it is a useless > feature. It is just that I found the message ' See error report: %s' > confusing in those cases where the report does not contain the error > at all because it is too short (the error report consists simply of the > last 400 lines of the build log). > > To answer your question, I believe there is always a build log - no > matter whether the `--log` option is used or not. In this case, removing > the error report and pointing to the build log in case of error (for > example like ' See build log: %s') would certainly solve all my concerns. > But on the build@ reports, it is nice to have something. Many times it is possible to diagnose the issue. Just in the past fifteen minutes, there was one which having the log made it clear that CentOS 7 and other older distributions need to use a newer GCC. Having the info in the build@ message was more than enough to diagnose tha
Re: [PATCH v1] move isr_level of SMP_lock_Context into ISR_lock_Context
On Mon, Jan 16, 2023 at 2:47 PM Sebastian Huber wrote: > > On 16.01.23 07:33, Zhu Zhongjie wrote: > > From: Zhongjie Zhu > > > > when Acquires an ISR lock in SMP system, it first call _ISR_Local_disable() > > to disable ISR, this is the same with in UP system, so we could > > use the same isr_level in ISR_lock_Context, this change will also save 4 > > Bytes > > of SMP_lock_Context. > > I don't think we should change the existing implementation which > separates the uniprocessor and SMP-specific parts. This patch mixes > things together. > > It is not necessary to save four bytes in SMP_lock_Context since this > structure is almost always allocated on the stack. > Sorry, I misunderstand the ISR_lock and _ISR_Local_disable I just thought those two smp related functions _SMP_lock_ISR_disable_and_acquire() and the _SMP_lock_Release_and_ISR_enable() mixed the ISR_lock information with SMP_lock I have read the history commits of isr_lock and smp_lock. > -- > embedded brains GmbH > Herr Sebastian HUBER > Dornierstr. 4 > 82178 Puchheim > Germany > email: sebastian.hu...@embedded-brains.de > phone: +49-89-18 94 741 - 16 > fax: +49-89-18 94 741 - 08 > > Registergericht: Amtsgericht München > Registernummer: HRB 157899 > Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler > Unsere Datenschutzerklärung finden Sie hier: > https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] build: Format build items
On 16/1/2023 6:56 pm, Sebastian Huber wrote: > On 16.01.23 01:35, Chris Johns wrote: >> On 13/1/2023 1:54 am, Sebastian Huber wrote: >>> On 12.01.23 15:44, Kinsey Moore wrote: The other two patches look fine to me. The use of dump() that results in this patch does several things: * Removal of whitespace This is fine for whitespace at the base level of indentation. Whitespace within an indented block may be more important for readability. * Removal of comments This is not good as they are exclusively used to annotate manually ordered blocks of test result expectations * Rearrangement of items in alphabetical order In general, rearrangement of top-level sections is good. For indented sections specifically in tst*.yml, this is bad for the above reaso >>> One goal of the new build system was to be able to alter the data through >>> scripts. This requires that the build items are human and machine readable >>> and >>> writable. The Python YAML import/export does not preserve white space and >>> comments. >> Can someone edit the file and add a hex number? > > Yes, you can manually use whatever format is understood by the YAML loader. > When > you write the file with the YAML dumper, then the standard representation is > used. Are there python modules in rtems.git someone could import that reads and writes the YAML spec files? If not should we provide a module to do this? It could be `spec` so a user can use `import spec` after setting the import path. That is, if external scripts (and I encourage this) all used a common read and write type functionality the format consistency is relative to specific version of rtems.git being used. Updates become read then write. > I changed the representer to use the format attribute, see v2 of the patch: > > https://lists.rtems.org/pipermail/devel/2023-January/074094.html > I see and thanks. How many format strings would cover the majority of formats we have? I am wondering if `format:` is checked against a standard list and those are part of the "writer" support? For example `address`, `address32`, `hex64` etc? I am concerned about repeated common format strings being placed through all the spec files. Chris ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] build: Format build items
On 17.01.23 03:48, Chris Johns wrote: On 16/1/2023 6:56 pm, Sebastian Huber wrote: On 16.01.23 01:35, Chris Johns wrote: On 13/1/2023 1:54 am, Sebastian Huber wrote: On 12.01.23 15:44, Kinsey Moore wrote: The other two patches look fine to me. The use of dump() that results in this patch does several things: * Removal of whitespace This is fine for whitespace at the base level of indentation. Whitespace within an indented block may be more important for readability. * Removal of comments This is not good as they are exclusively used to annotate manually ordered blocks of test result expectations * Rearrangement of items in alphabetical order In general, rearrangement of top-level sections is good. For indented sections specifically in tst*.yml, this is bad for the above reaso One goal of the new build system was to be able to alter the data through scripts. This requires that the build items are human and machine readable and writable. The Python YAML import/export does not preserve white space and comments. Can someone edit the file and add a hex number? Yes, you can manually use whatever format is understood by the YAML loader. When you write the file with the YAML dumper, then the standard representation is used. Are there python modules in rtems.git someone could import that reads and writes the YAML spec files? If not should we provide a module to do this? It could be `spec` so a user can use `import spec` after setting the import path. That is, if external scripts (and I encourage this) all used a common read and write type functionality the format consistency is relative to specific version of rtems.git being used. Updates become read then write. The Python modules to work with specification items are in rtems-central.git. This repository contains also a format specification of the build items. We could add an action to a Github work flow to check the build item format for pull requests and commits. I changed the representer to use the format attribute, see v2 of the patch: https://lists.rtems.org/pipermail/devel/2023-January/074094.html I see and thanks. How many format strings would cover the majority of formats we have? I am wondering if `format:` is checked against a standard list and those are part of the "writer" support? For example `address`, `address32`, `hex64` etc? I am concerned about repeated common format strings being placed through all the spec files. The format string is a standard Python format string. This is easy to implement and flexible. We could replace this with a fixed set of formats. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel