Skip to content

Commit 546e1b7

Browse files
MivrMivr
committed
Do not generate rules for ignored packages
1 parent fe7091c commit 546e1b7

File tree

4 files changed

+1207
-1388
lines changed

4 files changed

+1207
-1388
lines changed

‎e2e/npm_translate_lock_platform/test.sh

Lines changed: 115 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ echo "Platform: $PLATFORM $ARCH"
2626
echo "Bazel platform: ${BAZEL_PLATFORM}_${BAZEL_ARCH}"
2727

2828
# Build first to ensure bazel-out directory exists
29-
echo ""
30-
echo "Building node_modules to generate bazel-out structure..."
31-
bazel build //:node_modules >/dev/null
29+
echo "Building node_modules..."
30+
bazel build //:node_modules >/dev/null 2>&1
3231

3332
# Find the bazel-out directory structure
3433
BAZEL_OUT_DIR=""
@@ -40,139 +39,173 @@ for potential_dir in "bazel-out/${BAZEL_PLATFORM}_${BAZEL_ARCH}-fastbuild" "baze
4039
done
4140

4241
if [[ -z "$BAZEL_OUT_DIR" ]]; then
43-
echo "❌ Could not find bazel-out directory for platform ${BAZEL_PLATFORM}_${BAZEL_ARCH}"
42+
echo "ERROR: Could not find bazel-out directory for platform ${BAZEL_PLATFORM}_${BAZEL_ARCH}"
4443
echo "Available bazel-out directories:"
4544
ls -la bazel-out/ 2>/dev/null || echo "No bazel-out directory found"
4645
exit 1
4746
fi
4847

4948
echo "Found bazel-out directory: $BAZEL_OUT_DIR"
5049

51-
# Function to check if a package has a fake package.json
52-
check_package_is_fake() {
50+
# Function to check if a package repository directory exists
51+
check_package_repository_exists() {
5352
local package_name="$1"
54-
local expected_fake="$2" # "true" or "false"
53+
local should_exist="$2" # "true" or "false"
5554

56-
echo "Checking $package_name (should be $([ "$expected_fake" = "true" ] && echo "fake" || echo "real"))..."
55+
# Check if repository directory exists in bazel-out
56+
local repo_path="$BAZEL_OUT_DIR/bin/external/npm__esbuild_${package_name}__0.16.17"
57+
local package_store_path="$BAZEL_OUT_DIR/bin/node_modules/.aspect_rules_js/@esbuild+${package_name}@0.16.17"
5758

58-
# Build the expected path
59-
local package_path="$BAZEL_OUT_DIR/bin/node_modules/.aspect_rules_js/@esbuild+${package_name}@0.16.17/node_modules/@esbuild/${package_name}/package.json"
59+
local repo_exists="false"
60+
local package_exists="false"
6061

61-
if [[ ! -f "$package_path" ]]; then
62-
echo " ❌ Package file not found: $package_path"
63-
return 1
62+
if [[ -d "$repo_path" ]]; then
63+
repo_exists="true"
64+
fi
65+
66+
if [[ -d "$package_store_path" ]]; then
67+
package_exists="true"
6468
fi
6569

66-
echo " 📁 Found: $package_path"
67-
68-
# Check if package.json contains _incompatible marker (indicates fake package)
69-
local is_fake="false"
70-
if grep -q "_incompatible" "$package_path" 2>/dev/null; then
71-
is_fake="true"
72-
echo " 🚫 Contains _incompatible marker (fake package)"
73-
74-
# Show the fake package content for verification
75-
echo " 📄 Fake package.json content:"
76-
cat "$package_path" | sed 's/^/ /'
70+
# Verify expectations
71+
if [[ "$should_exist" = "true" ]]; then
72+
if [[ "$repo_exists" = "true" || "$package_exists" = "true" ]]; then
73+
echo " PASS: $package_name (compatible platform)"
74+
return 0
75+
else
76+
echo " FAIL: $package_name missing but should exist (compatible platform)"
77+
return 1
78+
fi
7779
else
78-
echo " ✅ No _incompatible marker (real package)"
79-
80-
# Show basic info about real package
81-
if command -v jq >/dev/null 2>&1; then
82-
local name=$(jq -r '.name // "unknown"' "$package_path" 2>/dev/null)
83-
local version=$(jq -r '.version // "unknown"' "$package_path" 2>/dev/null)
84-
echo " 📦 Real package: $name@$version"
80+
if [[ "$repo_exists" = "false" && "$package_exists" = "false" ]]; then
81+
echo " PASS: $package_name correctly filtered (incompatible platform)"
82+
return 0
83+
else
84+
echo " FAIL: $package_name exists but should be filtered (incompatible platform)"
85+
return 1
8586
fi
8687
fi
88+
}
89+
90+
# Function to check generated repositories.bzl file
91+
check_repositories_bzl() {
92+
# Look for the generated repositories file
93+
local repos_file=""
94+
for potential_file in "bazel-bin/external/npm/repositories.bzl" "bazel-out/*/bin/external/npm/repositories.bzl"; do
95+
if [[ -f "$potential_file" ]]; then
96+
repos_file="$potential_file"
97+
break
98+
fi
99+
done
87100

88-
# Verify expectations
89-
if [[ "$expected_fake" = "$is_fake" ]]; then
90-
echo " ✅ Package type matches expectation"
101+
if [[ -z "$repos_file" ]]; then
102+
echo "WARNING: Could not find repositories.bzl file"
91103
return 0
104+
fi
105+
106+
# Count npm_import rules for platform-specific packages
107+
local incompatible_count=0
108+
109+
# Define incompatible packages for current platform
110+
local incompatible_packages=()
111+
if [[ "$BAZEL_PLATFORM" = "darwin" && "$BAZEL_ARCH" = "arm64" ]]; then
112+
incompatible_packages=("linux-x64" "win32-x64" "linux-arm64")
113+
elif [[ "$BAZEL_PLATFORM" = "linux" && "$BAZEL_ARCH" = "amd64" ]]; then
114+
incompatible_packages=("darwin-arm64" "win32-x64" "darwin-x64")
92115
else
93-
echo " ❌ Package type mismatch: expected $([ "$expected_fake" = "true" ] && echo "fake" || echo "real"), got $([ "$is_fake" = "true" ] && echo "fake" || echo "real")"
116+
# Generic check - just look for common incompatible ones
117+
incompatible_packages=("win32-x64")
118+
fi
119+
120+
for package in "${incompatible_packages[@]}"; do
121+
if grep -q "npm__esbuild_${package}__" "$repos_file"; then
122+
echo "FAIL: Found npm_import rule for incompatible package: $package"
123+
incompatible_count=$((incompatible_count + 1))
124+
fi
125+
done
126+
127+
if [[ "$incompatible_count" -gt 0 ]]; then
128+
echo "FAIL: Found $incompatible_count npm_import rules for incompatible packages"
94129
return 1
130+
else
131+
echo "PASS: No npm_import rules found for incompatible packages"
132+
return 0
95133
fi
96134
}
97135

98136
# Main test logic based on current platform
99-
echo ""
100137
echo "Running platform-specific validation..."
101138

102139
success=true
103140

104141
if [[ "$BAZEL_PLATFORM" = "darwin" && "$BAZEL_ARCH" = "arm64" ]]; then
105-
echo "🖥️ Testing on Darwin ARM64..."
142+
# linux-x64 should NOT exist (incompatible)
143+
if ! check_package_repository_exists "linux-x64" "false"; then
144+
success=false
145+
fi
106146

107-
# linux-x64 should be fake (incompatible)
108-
if ! check_package_is_fake "linux-x64" "true"; then
147+
# win32-x64 should NOT exist (incompatible)
148+
if ! check_package_repository_exists "win32-x64" "false"; then
109149
success=false
110150
fi
111151

112-
# darwin-arm64 should be real (compatible) - but might be fake if optional
113-
echo ""
114-
if ! check_package_is_fake "darwin-arm64" "false"; then
115-
echo " ℹ️ darwin-arm64 is fake - this might be OK if it's optional and not needed"
116-
# Don't fail the test for this case
152+
# darwin-arm64 should exist (compatible) if not optional
153+
if ! check_package_repository_exists "darwin-arm64" "true"; then
154+
echo " INFO: darwin-arm64 doesn't exist - this is OK if it's optional and not needed"
155+
# Don't fail the test for this case since it's optional
117156
fi
118157

119158
elif [[ "$BAZEL_PLATFORM" = "linux" && "$BAZEL_ARCH" = "amd64" ]]; then
120-
echo "🖥️ Testing on Linux x64..."
159+
# darwin-arm64 should NOT exist (incompatible)
160+
if ! check_package_repository_exists "darwin-arm64" "false"; then
161+
success=false
162+
fi
121163

122-
# darwin-arm64 should be fake (incompatible)
123-
if ! check_package_is_fake "darwin-arm64" "true"; then
164+
# win32-x64 should NOT exist (incompatible)
165+
if ! check_package_repository_exists "win32-x64" "false"; then
124166
success=false
125167
fi
126168

127-
# linux-x64 should be real (compatible)
128-
echo ""
129-
if ! check_package_is_fake "linux-x64" "false"; then
130-
echo " ℹ️ linux-x64 is fake - this might be OK if it's optional and not needed"
131-
# Don't fail the test for this case
169+
# linux-x64 should exist (compatible) if not optional
170+
if ! check_package_repository_exists "linux-x64" "true"; then
171+
echo " INFO: linux-x64 doesn't exist - this is OK if it's optional and not needed"
172+
# Don't fail the test for this case since it's optional
132173
fi
133174

134175
else
135-
echo "🖥️ Testing on $BAZEL_PLATFORM $BAZEL_ARCH..."
136-
echo "ℹ️ Platform-specific validation not implemented for this platform"
137-
echo "ℹ️ Will check that at least one platform-specific package exists and some are fake"
138-
139-
# Generic test - just check that we have some fake packages
140-
fake_count=0
141-
total_count=0
142-
143-
for package in "linux-x64" "darwin-arm64"; do
144-
package_path="$BAZEL_OUT_DIR/bin/node_modules/.aspect_rules_js/@esbuild+${package}@0.16.17/node_modules/@esbuild/${package}/package.json"
145-
if [[ -f "$package_path" ]]; then
146-
total_count=$((total_count + 1))
147-
if grep -q "_incompatible" "$package_path" 2>/dev/null; then
148-
fake_count=$((fake_count + 1))
149-
echo " 🚫 Found fake package: $package"
150-
else
151-
echo " ✅ Found real package: $package"
152-
fi
176+
echo "Testing generic platform filtering..."
177+
178+
# Generic test - just check that some packages don't exist
179+
skipped_count=0
180+
total_checked=0
181+
182+
for package in "win32-x64" "win32-ia32" "sunos-x64"; do
183+
total_checked=$((total_checked + 1))
184+
if ! check_package_repository_exists "$package" "false"; then
185+
# Package exists when it shouldn't
186+
echo " WARNING: Package $package exists but should be filtered"
187+
else
188+
skipped_count=$((skipped_count + 1))
153189
fi
154190
done
155191

156-
echo " 📊 Summary: $fake_count fake packages out of $total_count total"
157-
158-
if [[ "$fake_count" -gt 0 ]]; then
159-
echo " 🎉 Platform filtering is working (found fake packages)"
160-
else
161-
echo " ⚠️ No fake packages found - platform filtering might not be working"
192+
if [[ "$skipped_count" -eq 0 ]]; then
193+
echo " FAIL: No packages filtered - platform filtering might not be working"
162194
success=false
163195
fi
164196
fi
165197

198+
# Check the generated repositories.bzl file
199+
if ! check_repositories_bzl; then
200+
success=false
201+
fi
202+
166203
# Final result
167204
echo ""
168-
echo "=== Test Summary ==="
169205
if [[ "$success" = "true" ]]; then
170-
echo "🎉 Platform filtering test passed!"
171-
echo "✅ Incompatible packages have fake package.json files"
172-
echo "✅ Platform-specific handling is working correctly"
206+
echo "PASS: Platform filtering test passed"
173207
exit 0
174208
else
175-
echo "❌ Platform filtering test failed!"
176-
echo "💡 Check that incompatible packages are being replaced with fake packages"
209+
echo "FAIL: Platform filtering test failed"
177210
exit 1
178211
fi

‎npm/private/npm_import.bzl

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -612,110 +612,8 @@ def _download_and_extract_archive(rctx, package_json_only):
612612
msg = "Failed to set directory listing permissions. '{}' exited with {}: \nSTDOUT:\n{}\nSTDERR:\n{}".format(" ".join(chmod_args), result.return_code, result.stdout, result.stderr)
613613
fail(msg)
614614

615-
def _is_package_compatible_with_current_platform(rctx, package_os, package_cpu):
616-
"""Check if a package is compatible with the current execution platform.
617-
618-
Args:
619-
rctx: repository context
620-
package_os: OS constraint from package metadata (string, list, or None)
621-
package_cpu: CPU constraint from package metadata (string, list, or None)
622-
623-
Returns:
624-
bool: True if compatible or no constraints, False if incompatible
625-
"""
626-
# Get normalized current platform info
627-
current_os, current_cpu = get_normalized_platform(rctx.os.name, rctx.os.arch)
628-
629-
return is_package_compatible_with_platform(package_os, package_cpu, current_os, current_cpu)
630-
631-
def _create_incompatible_package_fake(rctx, package_os, package_cpu):
632-
"""Create a fake package repository for incompatible packages.
633-
634-
This creates only minimal files to reduce system load while maintaining Bazel compatibility.
635-
This prevents downloads while providing clear indication of incompatibility.
636-
"""
637-
# Get normalized platform info for consistent reporting
638-
current_os, current_cpu = get_normalized_platform(rctx.os.name, rctx.os.arch)
639-
640-
# Normalize constraint display for clarity
641-
def format_constraint(constraint):
642-
if not constraint:
643-
return None
644-
return constraint if type(constraint) == "list" else [constraint]
645-
646-
required_os = format_constraint(package_os)
647-
required_cpu = format_constraint(package_cpu)
648-
649-
# Generate human-readable constraint description
650-
constraints = []
651-
if required_os:
652-
constraints.append("os={}".format(",".join(required_os)))
653-
if required_cpu:
654-
constraints.append("cpu={}".format(",".join(required_cpu)))
655-
656-
constraint_desc = " and ".join(constraints) if constraints else "specific platform constraints"
657-
658-
# Create a structured package.json with clear compatibility information
659-
package_json_content = {
660-
"name": rctx.attr.package,
661-
"version": rctx.attr.version,
662-
"description": "Platform-incompatible package placeholder created by rules_js",
663-
"_rules_js": {
664-
"incompatible": True,
665-
"reason": "platform_mismatch",
666-
"required": {
667-
"os": required_os,
668-
"cpu": required_cpu,
669-
},
670-
"current": {
671-
"os": current_os,
672-
"cpu": current_cpu,
673-
},
674-
"message": "This package requires {} but current platform is {}/{}".format(
675-
constraint_desc,
676-
current_os,
677-
current_cpu
678-
),
679-
},
680-
# Keep legacy marker for backward compatibility with existing tools/tests
681-
"_incompatible": "Platform mismatch: requires {} but current platform is {}/{}".format(
682-
constraint_desc,
683-
current_os,
684-
current_cpu
685-
),
686-
}
687-
rctx.file("package.json", json.encode_indent(package_json_content, indent = " "))
688-
689-
# Create a minimal BUILD.bazel file with required pkg target
690-
# Use npm_package_internal to provide proper providers but keep it minimal
691-
generated_by_prefix = _make_generated_by_prefix(rctx.attr.package, rctx.attr.version)
692-
build_content = """{generated_by_prefix}load("@aspect_rules_js//npm/private:npm_package_internal.bzl", _npm_package_internal = "npm_package_internal")
693-
694-
# Minimal fake package for platform-incompatible dependency
695-
_npm_package_internal(
696-
name = "pkg",
697-
src = ".",
698-
package = "{package}",
699-
version = "{version}",
700-
visibility = ["//visibility:public"],
701-
)
702-
""".format(
703-
generated_by_prefix = generated_by_prefix,
704-
package = rctx.attr.package,
705-
version = rctx.attr.version
706-
)
707-
rctx.file("BUILD.bazel", build_content)
708615

709616
def _npm_import_rule_impl(rctx):
710-
# Check platform compatibility early to avoid unnecessary downloads
711-
package_os = rctx.attr.os if rctx.attr.os else None
712-
package_cpu = rctx.attr.cpu if rctx.attr.cpu else None
713-
714-
if not _is_package_compatible_with_current_platform(rctx, package_os, package_cpu):
715-
# Package is incompatible - create fake target (no download but buildable)
716-
_create_incompatible_package_fake(rctx, package_os, package_cpu)
717-
return
718-
# else package is compatible - proceed with normal download
719617

720618
has_lifecycle_hooks = not (not rctx.attr.lifecycle_hooks) or not (not rctx.attr.custom_postinstall)
721619
has_patches = not (not rctx.attr.patches)

0 commit comments

Comments
 (0)