Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixes #619 help go mod vendor to include files #620

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Jan 28, 2025

Description

Fixes #619 .

Idea is just to hint to go mod vendor to include some assembly file by making it believe the subfolders in asm are actual go packages.
(in fact they just contains dummy constants and code that won't be linked, but the .s files are "included" à-la-C-preprocessor into other .s files)

@gbotrel gbotrel requested review from ivokub and Tabaie January 28, 2025 18:13
@ivokub
Copy link
Collaborator

ivokub commented Jan 31, 2025

Suggested edit:

diff --git a/field/asm/.gitignore b/field/asm/.gitignore
index d21c226a7..d9d3470d3 100644
--- a/field/asm/.gitignore
+++ b/field/asm/.gitignore
@@ -1,5 +1,7 @@
 # generated by integration tests
-element_2w/**
-element_3w/**
-element_7w/**
-element_8w/**
\ No newline at end of file
+element_4w/**
+element_5w/**
+element_6w/**
+element_10w/**
+element_12w/**
+element_31b/**
\ No newline at end of file

@ivokub
Copy link
Collaborator

ivokub commented Jan 31, 2025

Suggested edit:

diff --git a/field/generator/asm/amd64/build.go b/field/generator/asm/amd64/build.go
index 41219d576..a63a1f6cb 100644
--- a/field/generator/asm/amd64/build.go
+++ b/field/generator/asm/amd64/build.go
@@ -284,7 +284,7 @@ func ElementASMFileName(nbWords, nbBits int) string {
 	const fWN = "element_%dw"
 
 	if nbWords == 1 {
-		return filepath.Join(fmt.Sprintf(fW1, 31), fmt.Sprintf(nameW1, 31))
+		return filepath.Join(fmt.Sprintf(fW1, nbBits), fmt.Sprintf(nameW1, nbBits))
 	}
 	return filepath.Join(fmt.Sprintf(fWN, nbWords), fmt.Sprintf(nameWN, nbWords))
 }
@@ -294,7 +294,7 @@ func ElementASMBaseDir(nbWords, nbBits int) string {
 	const fWN = "element_%dw"
 
 	if nbWords == 1 {
-		return fmt.Sprintf(fW1, 31)
+		return fmt.Sprintf(fW1, nbBits)
 	}
 	return fmt.Sprintf(fWN, nbWords)
 }
diff --git a/field/generator/asm/arm64/build.go b/field/generator/asm/arm64/build.go
index d6f035551..1fb96c9f1 100644
--- a/field/generator/asm/arm64/build.go
+++ b/field/generator/asm/arm64/build.go
@@ -220,7 +220,7 @@ func ElementASMFileName(nbWords, nbBits int) string {
 	const fW1 = "element_%db"
 	const fWN = "element_%dw"
 	if nbWords == 1 {
-		return filepath.Join(fmt.Sprintf(fW1, 31), fmt.Sprintf(nameW1, 31))
+		return filepath.Join(fmt.Sprintf(fW1, nbBits), fmt.Sprintf(nameW1, nbBits))
 	}
 	return filepath.Join(fmt.Sprintf(fWN, nbWords), fmt.Sprintf(nameWN, nbWords))
 }

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

See suggests and a single comment. Otherwise looks perfect. I also tried out in an empty project and solves the issue.

@gbotrel
Copy link
Collaborator Author

gbotrel commented Jan 31, 2025

Suggested edit:

diff --git a/field/asm/.gitignore b/field/asm/.gitignore
index d21c226a7..d9d3470d3 100644
--- a/field/asm/.gitignore
+++ b/field/asm/.gitignore
@@ -1,5 +1,7 @@
 # generated by integration tests
-element_2w/**
-element_3w/**
-element_7w/**
-element_8w/**
\ No newline at end of file
+element_4w/**
+element_5w/**
+element_6w/**
+element_10w/**
+element_12w/**
+element_31b/**
\ No newline at end of file

can't do, we need 4w etc they are imported :-)

@gbotrel gbotrel merged commit 3de437d into master Jan 31, 2025
2 checks passed
@gbotrel gbotrel deleted the fix/vendorinclude branch January 31, 2025 15:05
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.

bug: broken vendoring in gnark-crypto@0.15.0
2 participants