[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17769177#comment-17769177 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/26/23 1:07 PM: I dont think this is the right direction - to still enable people to commit the test-related code to the production code base. That is the very reason we hit this issue - because we made it possible to depend on test-related libraries so this went undetected for, god ... 6 years? My idea is to flat out make it impossible to do so we do not need to fix this now and the code will not atrophy slowly anymore. was (Author: smiklosovic): I dont think this is the right direction - to still enable people to commit the test-related code to the production code base. That is the very reason we hit this issue - because we made it possible to depend on test-related libraries so this went undetected for, god ... 6 years? My idea is to flat out make it impossible to do so we do not need to fix this now and the code might just slowly atrophy ... > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 4.0.x, 4.1.x, 5.x > > Time Spent: 10m > Remaining Estimate: 0h > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17769155#comment-17769155 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/26/23 12:31 PM: - That's why we are looking into various directories to find the libs. If we put that to "build/lib/jars" it would find it there too. But we do not want to put it there because people can commit that to production "src". it does not matter it is not invoked anywhere etc. Technically it has nothing to do there. It is a test related source code. If somebody puts "TestRateLimiter" class to prod sources and he extends that from a byteman class, that is pretty much a source code supposed to be used for tests only. was (Author: smiklosovic): That's why we are looking into various directories to find the libs. If we put that to "build/lib/jars" it would find it there too. But we do not want to put it there because people can commit that to production "src". it does not matter it is not invoked anywhere etc. Technically it has nothing to do there. It is a test related source code. > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 4.0.x, 4.1.x, 5.x > > Time Spent: 10m > Remaining Estimate: 0h > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17769131#comment-17769131 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/26/23 11:50 AM: - But we will have byte buddy and byteman on the production class path? Why is that so problematic to patch, especially when it is backward compatible? Nothing will prevent a developer to put to prod code some byteman / bytebuddy annotated classes as we see here. was (Author: smiklosovic): But we will have byte buddy and byteman on the production class path? Why is that so problematic to patch, especially when it is backward compatible? > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 4.0.x, 4.1.x, 5.x > > Time Spent: 10m > Remaining Estimate: 0h > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17769131#comment-17769131 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/26/23 11:50 AM: - But we will have byte buddy and byteman on the production class path? Why is that so problematic to patch, especially when it is backward compatible? Nothing will prevent a developer from putting to prod code some byteman / bytebuddy annotated classes as we see here. was (Author: smiklosovic): But we will have byte buddy and byteman on the production class path? Why is that so problematic to patch, especially when it is backward compatible? Nothing will prevent a developer to put to prod code some byteman / bytebuddy annotated classes as we see here. > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 4.0.x, 4.1.x, 5.x > > Time Spent: 10m > Remaining Estimate: 0h > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17769088#comment-17769088 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/26/23 9:57 AM: We need to fix CCM (1). When we moved it to build/lib/test/jars (as it is test scoped), then CCM would not find it because it was expecting that in build/lib/jars instead. That was probably the reason why it was left there ... Byteman is used in dtests as well and it need agent to be run with Cassandra process and we need to submit the scripts to that. I made it backward compatible, it also works as it was. I will run all branches and update this ticket. (1) https://github.com/riptano/ccm/pull/757/files was (Author: smiklosovic): We need to fix CCM (1). When we moved it to build/lib/test/jars (as it is test scoped), then CCM would not find it because it was expecting that in build/lib/jars instead. That was probably the reason why it was left there ... I made it backward compatible, it also works as it was. I will run all branches and update this ticket. (1) https://github.com/riptano/ccm/pull/757/files > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 4.0.x, 4.1.x, 5.x > > Time Spent: 10m > Remaining Estimate: 0h > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-18877) remove bytebuddy / byteman from production classpath and remove compress-lzf dependency from build deps
[ https://issues.apache.org/jira/browse/CASSANDRA-18877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17768416#comment-17768416 ] Stefan Miklosovic edited comment on CASSANDRA-18877 at 9/24/23 4:34 PM: In (1), it was using LZFOutputStream in StreamWriter which just got removed. That library was also purposefuly removed from the repository. I am not sure how it got back but it is suspicious that it was never removed from the dependency management (2) and then when Maven / Ant resolver stuff was introduced it was probably just resurrected because of that. (1) https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb (2) https://github.com/apache/cassandra/blob/fc92db2b9b56c143516026ba29cecdec37e286bb/build.xml#L362 was (Author: smiklosovic): In (1), it was using LZFOutputStream in StreamWriter which just got removed. That library was also purposefuly removed from the repository. I am not sure how it got back but it is suspicious that it was never removed from the dependency management (2) and then when Maven / Ant resolver stuff was introduced it was just resurrected. (1) https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb (2) https://github.com/apache/cassandra/blob/fc92db2b9b56c143516026ba29cecdec37e286bb/build.xml#L362 > remove bytebuddy / byteman from production classpath and remove compress-lzf > dependency from build deps > --- > > Key: CASSANDRA-18877 > URL: https://issues.apache.org/jira/browse/CASSANDRA-18877 > Project: Cassandra > Issue Type: Task > Components: Build >Reporter: Stefan Miklosovic >Assignee: Stefan Miklosovic >Priority: Normal > Fix For: 5.x > > > I was digging in the project deps and if you compare all libs in "libs" dir > and all libs in "build/lib/jars", there are indeed some differences which are > OK however in build/lib/jars there are also libraries for byteman and > byte-buddy. This is clearly wrong as these dependecies should not be > accessible from the production code, only from tests. > The reason they are accessible in prod code is that there is the class > TestRateLimiter (1). I do not have a clue why that class is in the prod code > in the first place. The only place it is referenced in is here (2) but that > byteman script is not loaded anywhere in tests. I was also checking Python > dtests. > I think this is some leftover or something like "I will keep it here when I > need it", but as nobody seems to do, I strongly advocate for removing it and > making bytebuddy and byteman only test scoped dependencies as it should be. > A reader who pays attention notices that these dependencies are of provided > scope which is a trick to have it compilable but not among the libraries in > the production runtime and it does not do any harm as it is never invoked > from the production code (if it was, it would fail on missing imports) > neverthless this is still an issue which should be addressed. We were doing > something similar with assertj dependency recently. > The second issue is that there is a dependency on compress-lzf in build > dependencies. This is not necessary either as that library was removed from > the repository in (3) but it still somehow leaked to the build process again. > (1) > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/TestRateLimiter.java > (2) > https://github.com/apache/cassandra/blob/trunk/test/resources/byteman/mutation_limiter.btm > (3) > https://github.com/apache/cassandra/commit/fc92db2b9b56c143516026ba29cecdec37e286bb -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
