Improve wcurl version comparison mechanism#80
Conversation
|
I'm marking this as a draft because I still want to implement some testcases. |
0b56031 to
1d65f7e
Compare
It may be important to be able to specify a different curl binary to be invoked (e.g., when performing tests against different versions of curl, or when curl isn't installed in PATH). This commit introduces a new "--curl-binary" option. If it's not specified, wcurl defaults to using "curl". Signed-off-by: Sergio Durigan Junior <sergiodj@sergiodj.net>
We've been having a some problems with version comparison in the script. The approach we've taken so far is to decompose the current curl version into two variables (major and minor), and then perform comparisons against these components separately. It works, but it's confusing. Another possible approach would be to use a C-style version normalization (basically a printf "%02d%02d%02d") and then perform comparisons against the versions we want. The problem is that these versions need also to be normalized, which can be confusing as well. I decided to implement the second approach but abstract it as a simple function that can take a regular version string like "8.16.0" as well as a comparison operator that will then be passed onto to "test". This reads much nicer and abstracts the complexities of version normalization away. Unfortunately due to the limitations of shell scripting it's not easy to deduplicate code in this scenario, but that should be OK for now. Signed-off-by: Sergio Durigan Junior <sergiodj@sergiodj.net>
1d65f7e to
87ec4a9
Compare
|
OK, I'm happy enough to consider this ready to be reviewed. I had to implement a new Hopefully the commits are self-explanatory and the code isn't too hard to understand, but I'm happy to explain anything that might come up during review. |
|
@sergiodj Thanks for the PR and the improvements! Agreed this solution reads |
We've been having a some problems with version comparison in the script. The approach we've taken so far is to decompose the current curl version into two variables (major and minor), and then perform comparisons against these components separately. It works, but it's confusing.
Another possible approach would be to use a C-style version normalization (basically a
printf "%02d%02d") and then perform comparisons against the versions we want. The problem is that these versions need also to be normalized, which can be confusing as well.I decided to implement the second approach but abstract it as a simple function that can take a regular version string like
8.16as well as a comparison operator that will then be passed onto totest. This reads much nicer and abstracts the complexities of version normalization away. Unfortunately due to the limitations of shell scripting it's not easy to deduplicate code in this scenario, but that should be OK for now.