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.

Sign up for GitHub

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

Jump to bottom

Nested external FFI files are not bundled when publishing #4238

Open
ghivert opened this issue Feb 12, 2025 · 3 comments · May be fixed by #4256
Open

Nested external FFI files are not bundled when publishing #4238

ghivert opened this issue Feb 12, 2025 · 3 comments · May be fixed by #4256
Labels
bug Something isn't working good first issue Good for newcomers help wanted Contributions encouraged priority:high

Comments

@ghivert
Copy link

ghivert commented Feb 12, 2025

Hi!

When publishing a package with JS external FFI files not at root, they are not bundled in the output tarball.
The bug can easily be reproduced with. the following steps:

git clone [email protected]:ghivert/grille-pain.git
cd grille_pain
git checkout e1fa1168d5b6306cd134005721c78cac2bfad7e4
gleam publish

You should have the following display:

Source files:
  - LICENCE
  - README.md
  - gleam.toml
  - src/grille_pain/error.gleam
  - src/grille_pain/internals/css.gleam
  - src/grille_pain/internals/data/model.gleam
  - src/grille_pain/internals/data/msg.gleam
  - src/grille_pain/internals/data/toast.gleam
  - src/grille_pain/internals/effect_manager.gleam
  - src/grille_pain/internals/element.gleam
  - src/grille_pain/internals/global.gleam
  - src/grille_pain/internals/lustre/schedule.gleam
  - src/grille_pain/internals/setup.gleam
  - src/grille_pain/internals/shadow.gleam
  - src/grille_pain/internals/unsafe.gleam
  - src/grille_pain/internals/view/progress_bar.gleam
  - src/grille_pain/internals/view/theme.gleam
  - src/grille_pain/internals/view.gleam
  - src/grille_pain/lustre/toast.gleam
  - src/grille_pain/options.gleam
  - src/grille_pain/toast/level.gleam
  - src/grille_pain/toast.gleam
  - src/grille_pain.gleam

Name: grille_pain
Version: 1.1.1

Do you wish to publish this package? [y/n]:

Where you can see that every JS files are missing.
@ghivert ghivert added the bug Something isn't working label Feb 12, 2025
@PgBiel
Copy link
Contributor

PgBiel commented Feb 17, 2025

Indeed, I forgot to change this, sorry. Problem is in fn native_files and fn erlang_files:

gleam/compiler-cli/src/fs.rs

Lines 405 to 413 in dcf6bf9

pub fn native_files(dir: &Utf8Path) -> Result<impl Iterator<Item = Utf8PathBuf> + '_> {
Ok(read_dir(dir)?
.flat_map(Result::ok)
.map(|e| e.into_path())
.filter(|path| {
let extension = path.extension().unwrap_or_default();
matches!(extension, "erl" | "hrl" | "ex" | "js" | "mjs" | "ts")
}))
}

gleam/compiler-cli/src/fs.rs

Lines 426 to 434 in dcf6bf9

pub fn erlang_files(dir: &Utf8Path) -> Result<impl Iterator<Item = Utf8PathBuf> + '_> {
Ok(read_dir(dir)?
.flat_map(Result::ok)
.map(|e| e.into_path())
.filter(|path| {
let extension = path.extension().unwrap_or_default();
extension == "erl" || extension == "hrl"
}))
}

They should recursively search the src folder.

Also, they should probably function similarly to the other functions, excluding .gitignored files.

@PgBiel
Copy link
Contributor

PgBiel commented Feb 17, 2025 •
edited
Loading

I can take a stab at this, though it's not very clear how to add automated tests for it. (Edit: PR created with some tests: #4256)

@PgBiel PgBiel linked a pull request Feb 18, 2025 that will close this issue
Draft
@lpil lpil added help wanted Contributions encouraged good first issue Good for newcomers priority:high labels Feb 18, 2025
@lpil
Copy link
Member

lpil commented Feb 18, 2025

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good first issue Good for newcomers help wanted Contributions encouraged priority:high
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bundle nested FFI on publish PgBiel/gleam
3 participants
@lpil @ghivert @PgBiel