You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "fresh-borzoni (via GitHub)" <gi...@apache.org> on 2026/08/31 14:07:49 UTC

[PR] [rust] Verify the dependency inventory in CI [fluss]

fresh-borzoni opened a new pull request, #4169:
URL: https://github.com/apache/fluss/pull/4169

   The DEPENDENCIES.rust.tsv files ship inside the fluss-rust source release, but nothing kept them current and  the license job only checks the live dependency tree, never the checked-in file.
   
   This regenerates them and makes the license job fail when they drift, so scripts/dependencies.py gains a verify`subcommand for it and asserts the cargo-deny version first. It also checks the source tarball still carries LICENSE, NOTICE and the inventory at its root.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #4169:
URL: https://github.com/apache/fluss/pull/4169#discussion_r3895443622


##########
.github/workflows/rust-license-and-format.yml:
##########


Review Comment:
   Should we include the paths to scripts, DEPENDENCIES.rust.tsv, LICENCE and NOTICE here?



##########
.github/workflows/rust-license-and-format.yml:
##########


Review Comment:
   Should we include the paths to scripts, DEPENDENCIES.rust.tsv, LICENCE and NOTICE here?



##########
fluss-rust/scripts/dependencies.py:
##########
@@ -26,53 +26,115 @@
         f"Current: {sys.version}. Use python3.11+ or see docs for release requirements."
     )
 
+import difflib
 import subprocess
 from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
+from functools import cache
 
 from constants import PACKAGES, ROOT_DIR
 
+# Must match the pin in .github/workflows/rust-license-and-format.yml: the
+# generated report differs between cargo-deny versions.
+CARGO_DENY_VERSION = "0.14.22"

Review Comment:
   Should we bump this version?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #4169:
URL: https://github.com/apache/fluss/pull/4169#issuecomment-5480159738

   @leekeiabstraction Thank you for the review addressed, PTAL 🙏 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni merged PR #4169:
URL: https://github.com/apache/fluss/pull/4169


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #4169:
URL: https://github.com/apache/fluss/pull/4169#discussion_r3896548351


##########
.github/workflows/gateway-ci.yml:
##########
@@ -134,7 +134,7 @@ jobs:
       - name: Install cargo-deny
         uses: taiki-e/install-action@v2
         with:
-          tool: cargo-deny@0.14.22
+          tool: cargo-deny@0.20.2

Review Comment:
   no, we don't need it, ci checked it :) 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #4169:
URL: https://github.com/apache/fluss/pull/4169#discussion_r3895796155


##########
fluss-rust/scripts/dependencies.py:
##########
@@ -26,53 +26,115 @@
         f"Current: {sys.version}. Use python3.11+ or see docs for release requirements."
     )
 
+import difflib
 import subprocess
 from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
+from functools import cache
 
 from constants import PACKAGES, ROOT_DIR
 
+# Must match the pin in .github/workflows/rust-license-and-format.yml: the
+# generated report differs between cargo-deny versions.
+CARGO_DENY_VERSION = "0.20.2"
 
-def check_single_package(root):
-    pkg_dir = ROOT_DIR / root if root != "." else ROOT_DIR
-    if (pkg_dir / "Cargo.toml").exists():
-        print(f"Checking dependencies of {root}")
-        subprocess.run(
-            ["cargo", "deny", "check", "license"],
-            cwd=pkg_dir,
-            check=True,
+
+@cache
+def verify_cargo_deny():
+    output = subprocess.check_output(
+        ["cargo", "deny", "--version"], cwd=ROOT_DIR, text=True
+    ).strip()
+    actual = output.rsplit(" ", 1)[-1]
+    if actual != CARGO_DENY_VERSION:
+        raise RuntimeError(
+            f"cargo-deny {CARGO_DENY_VERSION} is required, found {output!r}"
+        )
+
+
+def package_dir(root):
+    return ROOT_DIR / root if root != "." else ROOT_DIR
+
+
+def cargo_deny(root, *args, capture_output=False):
+    verify_cargo_deny()
+    return subprocess.run(
+        ["cargo", "deny", "--locked", "--all-features", *args],
+        cwd=package_dir(root),
+        check=not capture_output,
+        capture_output=capture_output,
+        text=True,
+    )
+
+
+def output_file(root):
+    return package_dir(root) / "DEPENDENCIES.rust.tsv"
+
+
+def dependency_report(root):
+    result = cargo_deny(root, "list", "-f", "tsv", "-t", "0.6", capture_output=True)
+    if result.returncode != 0:
+        raise RuntimeError(
+            f"cargo deny list failed in {root}: {result.stderr or result.stdout}"
         )
-    else:
-        print(f"Skipping {root} as Cargo.toml does not exist")
+    return result.stdout
+
+
+def for_each_package(action):
+    for root in PACKAGES:
+        if (package_dir(root) / "Cargo.toml").exists():
+            action(root)
+        else:
+            print(f"Skipping {root} as Cargo.toml does not exist")
+
+
+def check_single_package(root):
+    print(f"Checking dependencies of {root}")
+    cargo_deny(root, "check", "license")

Review Comment:
   i think license be a plural case actually? https://embarkstudios.github.io/cargo-deny/checks/licenses/index.html



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #4169:
URL: https://github.com/apache/fluss/pull/4169#discussion_r3896397489


##########
fluss-rust/scripts/dependencies.py:
##########
@@ -26,53 +26,115 @@
         f"Current: {sys.version}. Use python3.11+ or see docs for release requirements."
     )
 
+import difflib
 import subprocess
 from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
+from functools import cache
 
 from constants import PACKAGES, ROOT_DIR
 
+# Must match the pin in .github/workflows/rust-license-and-format.yml: the
+# generated report differs between cargo-deny versions.
+CARGO_DENY_VERSION = "0.20.2"
 
-def check_single_package(root):
-    pkg_dir = ROOT_DIR / root if root != "." else ROOT_DIR
-    if (pkg_dir / "Cargo.toml").exists():
-        print(f"Checking dependencies of {root}")
-        subprocess.run(
-            ["cargo", "deny", "check", "license"],
-            cwd=pkg_dir,
-            check=True,
+
+@cache
+def verify_cargo_deny():
+    output = subprocess.check_output(
+        ["cargo", "deny", "--version"], cwd=ROOT_DIR, text=True
+    ).strip()
+    actual = output.rsplit(" ", 1)[-1]
+    if actual != CARGO_DENY_VERSION:
+        raise RuntimeError(
+            f"cargo-deny {CARGO_DENY_VERSION} is required, found {output!r}"
+        )
+
+
+def package_dir(root):
+    return ROOT_DIR / root if root != "." else ROOT_DIR
+
+
+def cargo_deny(root, *args, capture_output=False):
+    verify_cargo_deny()
+    return subprocess.run(
+        ["cargo", "deny", "--locked", "--all-features", *args],
+        cwd=package_dir(root),
+        check=not capture_output,
+        capture_output=capture_output,
+        text=True,
+    )
+
+
+def output_file(root):
+    return package_dir(root) / "DEPENDENCIES.rust.tsv"
+
+
+def dependency_report(root):
+    result = cargo_deny(root, "list", "-f", "tsv", "-t", "0.6", capture_output=True)
+    if result.returncode != 0:
+        raise RuntimeError(
+            f"cargo deny list failed in {root}: {result.stderr or result.stdout}"
         )
-    else:
-        print(f"Skipping {root} as Cargo.toml does not exist")
+    return result.stdout
+
+
+def for_each_package(action):
+    for root in PACKAGES:
+        if (package_dir(root) / "Cargo.toml").exists():
+            action(root)
+        else:
+            print(f"Skipping {root} as Cargo.toml does not exist")
+
+
+def check_single_package(root):
+    print(f"Checking dependencies of {root}")
+    cargo_deny(root, "check", "license")

Review Comment:
   That's a very good catch, it was changed in the newer cargo-deny 👍 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #4169:
URL: https://github.com/apache/fluss/pull/4169#issuecomment-5479554295

   @leekeiabstraction PTAL 🙏 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [rust] Verify the dependency inventory in CI [fluss]

Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #4169:
URL: https://github.com/apache/fluss/pull/4169#discussion_r3895796155


##########
fluss-rust/scripts/dependencies.py:
##########
@@ -26,53 +26,115 @@
         f"Current: {sys.version}. Use python3.11+ or see docs for release requirements."
     )
 
+import difflib
 import subprocess
 from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
+from functools import cache
 
 from constants import PACKAGES, ROOT_DIR
 
+# Must match the pin in .github/workflows/rust-license-and-format.yml: the
+# generated report differs between cargo-deny versions.
+CARGO_DENY_VERSION = "0.20.2"
 
-def check_single_package(root):
-    pkg_dir = ROOT_DIR / root if root != "." else ROOT_DIR
-    if (pkg_dir / "Cargo.toml").exists():
-        print(f"Checking dependencies of {root}")
-        subprocess.run(
-            ["cargo", "deny", "check", "license"],
-            cwd=pkg_dir,
-            check=True,
+
+@cache
+def verify_cargo_deny():
+    output = subprocess.check_output(
+        ["cargo", "deny", "--version"], cwd=ROOT_DIR, text=True
+    ).strip()
+    actual = output.rsplit(" ", 1)[-1]
+    if actual != CARGO_DENY_VERSION:
+        raise RuntimeError(
+            f"cargo-deny {CARGO_DENY_VERSION} is required, found {output!r}"
+        )
+
+
+def package_dir(root):
+    return ROOT_DIR / root if root != "." else ROOT_DIR
+
+
+def cargo_deny(root, *args, capture_output=False):
+    verify_cargo_deny()
+    return subprocess.run(
+        ["cargo", "deny", "--locked", "--all-features", *args],
+        cwd=package_dir(root),
+        check=not capture_output,
+        capture_output=capture_output,
+        text=True,
+    )
+
+
+def output_file(root):
+    return package_dir(root) / "DEPENDENCIES.rust.tsv"
+
+
+def dependency_report(root):
+    result = cargo_deny(root, "list", "-f", "tsv", "-t", "0.6", capture_output=True)
+    if result.returncode != 0:
+        raise RuntimeError(
+            f"cargo deny list failed in {root}: {result.stderr or result.stdout}"
         )
-    else:
-        print(f"Skipping {root} as Cargo.toml does not exist")
+    return result.stdout
+
+
+def for_each_package(action):
+    for root in PACKAGES:
+        if (package_dir(root) / "Cargo.toml").exists():
+            action(root)
+        else:
+            print(f"Skipping {root} as Cargo.toml does not exist")
+
+
+def check_single_package(root):
+    print(f"Checking dependencies of {root}")
+    cargo_deny(root, "check", "license")

Review Comment:
   i think license be a plural case actually?



##########
.github/workflows/gateway-ci.yml:
##########
@@ -134,7 +134,7 @@ jobs:
       - name: Install cargo-deny
         uses: taiki-e/install-action@v2
         with:
-          tool: cargo-deny@0.14.22
+          tool: cargo-deny@0.20.2

Review Comment:
   there is also a dependency tsv in `fluss-gateway`, do we need regen that as well?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org