Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Release v0.27.0 #108

Merged
merged 8 commits into from
Sep 19, 2019
Merged

Release v0.27.0 #108

merged 8 commits into from
Sep 19, 2019

Conversation

bassosimone
Copy link
Member

@bassosimone bassosimone commented Aug 2, 2019

This PR aims to release v0.27.0 of libndt where the legacy protocol (which has 100% coverage and has received the most auditing) is ready to be included by MK. After measurement-kit/measurement-kit#1880 and trying to add hacks to make it work, I really started wishing we had replaced MK's NDT code with this code. This is tracked by measurement-kit/measurement-kit#1881.

@bassosimone bassosimone force-pushed the release/v0.27.0 branch 7 times, most recently from 4933541 to 36826fd Compare August 2, 2019 09:11
bassosimone added a commit to measurement-kit/mkbuild that referenced this pull request Aug 2, 2019
bassosimone added a commit to measurement-kit/mkbuild that referenced this pull request Aug 2, 2019
bassosimone added a commit to measurement-kit/mkbuild that referenced this pull request Aug 2, 2019
bassosimone added a commit to measurement-kit/mkbuild that referenced this pull request Aug 2, 2019
This diff converts libndt to use mkbuild as opposed to deprecated
subrepositories that we would like to decommission.

It includes uncommitted changes to mkbuild required to make this conversion
actually possible, see measurement-kit/mkbuild#6.

It also removes conditional support for OpenSSL and cURL mainly
because that simplifies the migration, _and_ because having such
support as optional was optional complexity.

This diff additionally changes the testing strategy in Travis
to only cover the legacy code path. It appears that there have
been changes again and now running several NDT tests from the
same invocation of Travis has become problematic.

This is reasonably okay because at this stage we're going to use
libndt only as an implementation of the legacy protocol.

Closes #91.

Closes #92.
@bassosimone
Copy link
Member Author

A blocker to moving forward this PR is a new failure more for Travis builds:

[127%] elapsed: 17.769 s; test_id: 2; num_flows: 1; speed:    307 Mbit/s
[!] netx_send: netx_send_nonblocking() failed: timed_out
[!] run_upload: sending: timed_out
running download test
[!] netx_dial: connect() failed: operation_in_progress
[!] netx_dial: connect() failed: io_error
[!] run_download: not all connect succeeded

The io_error error is a catch-all error in this case. It's some error I didn't map. In one spot test that I've lost track of, the errno value was 99 (i.e. EADDRNOTAVAIL). 🤔

@bassosimone
Copy link
Member Author

It seems the afore mentioned blocker is not a real blocker, rather a transient failure of travis that did not appear again when re-running integration tests after some time.

@bassosimone
Copy link
Member Author

bassosimone commented Aug 28, 2019

TODO list for merging this PR

@codecov-io
Copy link

Codecov Report

Merging #108 into master will decrease coverage by 9.91%.
The diff coverage is 29.41%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #108      +/-   ##
=========================================
- Coverage   86.01%   76.1%   -9.92%     
=========================================
  Files           5       5              
  Lines        2868    2871       +3     
=========================================
- Hits         2467    2185     -282     
- Misses        401     686     +285
Impacted Files Coverage Δ
libndt.hpp 62.38% <29.41%> (-18.72%) ⬇️
libndt-client.cpp 34.73% <0%> (-11.58%) ⬇️
libndt_test.cpp 93.94% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db33c1e...f7baf4f. Read the comment docs.

bassosimone added a commit to measurement-kit/mkbuild that referenced this pull request Sep 6, 2019
* Implement check_{function,symbol}_exists

Required by and used in measurement-kit/libndt#108

* macOS: force OpenSSL >= 1.1.1

Required by and used in measurement-kit/libndt#108

* docker/docker.go: go fmt

* cmake: add OpenSSL prebuilt for Windows

We'll be using a version of LibreSSL that we previously built
to have a working, albeit old, "OpenSSL" for Windows.

This is fine because we only use this prebuilt library for
running continuous integration tests.

Because LibreSSL/OpenSSL consists of multiple libraries, it
was necessary here to rework how prebuilt libs work.
@bassosimone
Copy link
Member Author

backport changes from measurement-kit/measurement-kit to emulate the network performance of cable connections, thus making travis impact negligible

Not enough bandwidth to do that now. Created new issue: measurement-kit/mkbuild#7.

@bassosimone bassosimone marked this pull request as ready for review September 13, 2019 16:07
Copy link
Member Author

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

Add some comments supposed to guide review

- cmd: .\.ci\common\script\appveyor.bat
- cmd: cmake -G "%CMAKE_GENERATOR%" .
- cmd: cmake --build . -- /nologo /property:Configuration=Release
- cmd: ctest --output-on-failure -C Release -a
Copy link
Member Author

Choose a reason for hiding this comment

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

This is exactly how we do it for github.com/measurement-kit/mkcurl

/x64/
/ZERO_CHECK.vcxproj
/ZERO_CHECK.vcxproj.filters
/build
Copy link
Member Author

Choose a reason for hiding this comment

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

Simplified like we go in github.com/measurement-kit/mkcurl

@@ -1,6 +0,0 @@
[submodule ".ci/common"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Submodules got removed

@@ -10,4 +10,4 @@ matrix:
- env: BUILD_TYPE="ubsan"
- env: BUILD_TYPE="vanilla"
script:
- ./.ci/common/script/travis $BUILD_TYPE
- ./docker.sh $BUILD_TYPE
Copy link
Member Author

Choose a reason for hiding this comment

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

Now using the script generated by mkbuild

@@ -1,46 +1,52 @@
cmake_minimum_required(VERSION 3.1.0)
Copy link
Member Author

Choose a reason for hiding this comment

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

This contains a great deal of mechanical changes

@bassosimone bassosimone added the enhancement New feature or request label Sep 13, 2019
@bassosimone bassosimone added this to the libndt-0.27.0 milestone Sep 13, 2019
@bassosimone bassosimone merged commit e463cab into master Sep 19, 2019
@bassosimone bassosimone deleted the release/v0.27.0 branch November 7, 2019 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants