Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common: add UBSAN build #5819

Merged
merged 1 commit into from
Jul 21, 2023
Merged

common: add UBSAN build #5819

merged 1 commit into from
Jul 21, 2023

Conversation

osalyk
Copy link
Contributor

@osalyk osalyk commented Jul 21, 2023

@osalyk osalyk added the sprint goal This pull request is part of the ongoing sprint label Jul 21, 2023
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #5819 (9774c76) into master (71d8e52) will not change coverage.
The diff coverage is n/a.

❗ Current head 9774c76 differs from pull request most recent head 4466953. Consider uploading reports for the commit 4466953 to get more accurate results

@@           Coverage Diff           @@
##           master    #5819   +/-   ##
=======================================
  Coverage   71.00%   71.00%           
=======================================
  Files         131      131           
  Lines       19175    19175           
  Branches     3193     3193           
=======================================
  Hits        13616    13616           
  Misses       5559     5559           

@janekmi janekmi added this to the 2.0.0 milestone Jul 21, 2023
Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @osalyk)


.github/workflows/scan_ubsan.yml line 9 at r1 (raw file):

env:

I believe this new line is redundant.


.github/workflows/scan_ubsan.yml line 38 at r1 (raw file):

        run: cd $WORKDIR && ./pull-or-rebuild-image.sh

      - name: Build CI with UBSAN

Suggestion:

Build libraries with Undefined Behavior Sanitizer and run tests

.github/workflows/scan_ubsan.yml line 39 at r1 (raw file):

      - name: Build CI with UBSAN
        run: cd $WORKDIR && TEST_BUILD=${{ matrix.build }} ./build-CI.sh

I prefer using the dedicated env syntax wherever it is possible.

Suggestion:

      - name: Build CI with UBSAN
        env:
          TEST_BUILD: ${{ matrix.build }}
        run: cd $WORKDIR && ./build-CI.sh

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @janekmi)


.github/workflows/scan_ubsan.yml line 9 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I believe this new line is redundant.

Done.


.github/workflows/scan_ubsan.yml line 38 at r1 (raw file):

        run: cd $WORKDIR && ./pull-or-rebuild-image.sh

      - name: Build CI with UBSAN

Done.


.github/workflows/scan_ubsan.yml line 39 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I prefer using the dedicated env syntax wherever it is possible.

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

@janekmi janekmi merged commit 0a39bd9 into pmem:master Jul 21, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sprint goal This pull request is part of the ongoing sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants