Skip to content

deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5#18474

Merged
phlax merged 3 commits intoenvoyproxy:mainfrom
phlax:bump-zlib
Oct 8, 2021
Merged

deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5#18474
phlax merged 3 commits intoenvoyproxy:mainfrom
phlax:bump-zlib

Conversation

@phlax
Copy link
Copy Markdown
Member

@phlax phlax commented Oct 6, 2021

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] Fix #18333
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Ryan Northey <ryan@synca.io>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #18474 was opened by phlax.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Oct 6, 2021
@phlax phlax changed the title deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5 [WIP] deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5 Oct 6, 2021
@phlax phlax marked this pull request as draft October 6, 2021 09:39
@moderation
Copy link
Copy Markdown
Contributor

I'm trying to recall why but in order for this update to work you'll need to change the patch

 bazel/foreign_cc/zlib_ng.patch | 21 +++++++++++----------                  
 1 file changed, 11 insertions(+), 10 deletions(-)                          
                                                                            
diff --git a/bazel/foreign_cc/zlib_ng.patch b/bazel/foreign_cc/zlib_ng.patch
index 77b04ef09..a5843014f 100644                                           
--- a/bazel/foreign_cc/zlib_ng.patch                                        
+++ b/bazel/foreign_cc/zlib_ng.patch                                        
@@ -1,13 +1,14 @@                                                           
+                                                                           
 # Add support for compiling to WebAssembly using Emscripten.               
 # https://github.com/zlib-ng/zlib-ng/pull/794                              
 diff --git a/cmake/detect-arch.c b/cmake/detect-arch.c                     
-index 5715535..2137691 100644                                              
+                                                                           
 --- a/cmake/detect-arch.c                                                  
 +++ b/cmake/detect-arch.c                                                  
-@@ -93,6 +93,10 @@                                                         
- #elif defined(__THW_RS6000)                                               
-     #error archfound rs6000                                               
-                                                                           
+@@ -101,6 +101,10 @@                                                       
+         #error archfound riscv32                                          
+     #endif                                                                
+                                                                           
 +// Emscripten (WebAssembly)                                               
 +#elif defined(__EMSCRIPTEN__)                                             
 +    #error archfound wasm32                                               
@@ -16,16 +17,16 @@ index 5715535..2137691 100644                           
  #else                                                                     
      #error archfound unrecognized                                         
 diff --git a/cmake/detect-arch.cmake b/cmake/detect-arch.cmake             
-index b80d666..c6cc214 100644                                              
+                                                                           
 --- a/cmake/detect-arch.cmake                                              
 +++ b/cmake/detect-arch.cmake                                              
-@@ -85,6 +85,9 @@ elseif("${ARCH}" MATCHES "parisc")                       
+@@ -85,6 +85,9 @@                                                          
  elseif("${ARCH}" MATCHES "rs6000")                                        
      set(BASEARCH "rs6000")                                                
      set(BASEARCH_RS6000_FOUND TRUE)                                       
 +elseif("${ARCH}" MATCHES "wasm32")                                        
 +    set(BASEARCH "wasm32")                                                
 +    set(BASEARCH_WASM32_FOUND TRUE)                                       
- else()                                                                    
-     set(BASEARCH "x86")                                                   
-     set(BASEARCH_X86_FOUND TRUE)                                          
+ elseif("${ARCH}" MATCHES "riscv(32|64)")                                  
+     set(BASEARCH "riscv")                                                 
+     set(BASEARCH_RISCV_FOUND TRUE)                                        

@phlax
Copy link
Copy Markdown
Member Author

phlax commented Oct 6, 2021

thanks ill update on the next go-round

i marked any that failed as WIP, some fail due to patch issues, but some i think because they need to be updated concurrently

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Oct 7, 2021

hmm, the provided patch applies cleanly but the patched patch does not

i must have been looking at the wrong ci run, seems to be going through...

@phlax phlax changed the title [WIP] deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5 deps: Bump com_github_zlib_ng_zlib_ng -> 2.0.5 Oct 7, 2021
@phlax phlax marked this pull request as ready for review October 7, 2021 10:29
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Oct 7, 2021

...and it fails the wasm un/compress tests in compile_time_options

compress 2000 -> 23
unknown file: Failure

Unexpected mock function call - returning directly.
    Function call: log_(0, "compress 2000 -> 23")
Google Mock tried the following 2 expectations, but none matched:

external/envoy/test/extensions/common/wasm/wasm_test.cc:573: tried expectation #0: EXPECT_CALL(*root_context, log_(spdlog::level::trace, Eq("compress 2000 -> 22")))...
  Expected arg #1: is equal to 0x2d13e5 pointing to "compress 2000 -> 22"
           Actual: "compress 2000 -> 23"
         Expected: to be called once
           Actual: never called - unsatisfied and active
external/envoy/test/extensions/common/wasm/wasm_test.cc:574: tried expectation #1: EXPECT_CALL(*root_context, log_(spdlog::level::debug, Eq("uncompress 22 -> 2000")))...
  Expected arg #0: is equal to 1
           Actual: 0
  Expected arg #1: is equal to 0x38fa16 pointing to "uncompress 22 -> 2000"
           Actual: "compress 2000 -> 23"
         Expected: to be called once
           Actual: never called - unsatisfied and active
Stack trace:
  0x8574a24: testing::internal::GoogleTestFailureReporter::ReportFailure()
  0x3211250: testing::internal::Expect()
  0x8576fe9: testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith()::$_1::operator()()
  0x8576c3a: testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith()
  0x31a5952: testing::internal::FunctionMocker<>::Invoke()
  0x31a585d: Envoy::Extensions::Common::Wasm::TestContext::log_()
  0x319c771: Envoy::Extensions::Common::Wasm::TestContext::log()
  0x61a2013: proxy_wasm::exports::log()
  0x61c78cc: proxy_wasm::ConvertFunctionWordToUint32<>::convertFunctionWordToUint32()
  0x327d4ee: std::__invoke_impl<>()
  0x327d47c: std::__invoke<>()
  0x327d3fc: std::__apply_impl<>()
  0x327d382: std::apply<>()
  0x327d15a: proxy_wasm::wamr::Wamr::registerHostFunctionImpl<>()::{lambda()#1}::operator()()
  0x327cee5: proxy_wasm::wamr::Wamr::registerHostFunctionImpl<>()::{lambda()#1}::__invoke()
  0x32a5390: wasm_runtime_invoke_c_api_native
  0x32d344e: aot_invoke_native
  0x40ef902b: (unknown)

uncompress 23 -> 2000
unknown file: Failure

@rojkov
Copy link
Copy Markdown
Member

rojkov commented Oct 8, 2021

...and it fails the wasm un/compress tests in compile_time_options

It seems the new version of the lib produces exactly the same compression ratio as the old zlib. Try to drop this condition:

#ifdef ZLIBNG_VERSION

@rojkov
Copy link
Copy Markdown
Member

rojkov commented Oct 8, 2021

/wait

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Oct 8, 2021

thanks, updated, lets see if it goes through...

@phlax phlax requested a review from moderation October 8, 2021 13:59
@moderation
Copy link
Copy Markdown
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Oct 8, 2021
Copy link
Copy Markdown
Member

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@phlax phlax merged commit 015678a into envoyproxy:main Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Newer release available com_github_zlib_ng_zlib_ng: 2.0.5

3 participants