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

forc panics if the Sway package folder contains dot (.) #5434

Closed
ironcev opened this issue Jan 4, 2024 · 0 comments · Fixed by #5448
Closed

forc panics if the Sway package folder contains dot (.) #5434

ironcev opened this issue Jan 4, 2024 · 0 comments · Fixed by #5448
Labels
bug Something isn't working forc good first issue Good for newcomers

Comments

@ironcev
Copy link
Member

ironcev commented Jan 4, 2024

Steps to reproduce:

$ mkdir path_with_._dot
$ cd path_with_._dot
path_with_._dot$ forc init --script
path_with_._dot$ forc build

Build will panic with the followinig message:

thread 'main' panicked at 'file_name must be either absolute or relative to manifest directory', sway-core/src/build_config.rs:74:17

After renaming the folder to a name without dot, the build will pass:

path_with_._dot$ cd ..
$ mv path_with_._dot path_without_dot
$ cd path_without_dot
path_without_dot$ forc build
@ironcev ironcev added bug Something isn't working forc labels Jan 4, 2024
@xunilrj xunilrj added the good first issue Good for newcomers label Jan 5, 2024
@Halimao Halimao mentioned this issue Jan 10, 2024
Merged
7 tasks
kayagokalp pushed a commit that referenced this issue Jan 11, 2024
@Halimao
fix: forc build panic when sway package folder contains dot (#5448)
5dda9a7 
## Description
Close #5434

Since `canonical_manifest_dir` is a dir, not a file, to get the
last-level dir name, we should use `file_name()` instead of `file_stem`.
@Halimao Halimao mentioned this issue Jan 11, 2024
Merged
7 tasks
sdankel added a commit that referenced this issue Mar 21, 2024
@Halimao @sdankel
refactor: forc init generate unexpected project_name when project d…
4d23798 
…ir contains dot (#5455)

## Description
Refer #5434

Found this issue when I tried to run `forc init --script` under a
directory(`/tmp/path_with_._dot`) that contains a dot ".". The value of
`project.name` in the generated file `Forc.toml` is truncated by the dot
"." as `path_with_`.

Since `project_dir` must be a directory(has checked before), we can use
`file_name` instead of `file_stem` to get the last-level dir name. Also,
I think it would be better to replace "." with "_", otherwise a
`project_name` that contains `.` would fail with validation
`validate_name(&project_name, "project name")?;`

## Screenshots

#### Before:

![image](https://github.com/FuelLabs/sway/assets/25278203/b6073a20-81ea-45cb-9b58-842e43cb3257)

#### After:

![image](https://github.com/FuelLabs/sway/assets/25278203/4c48fffd-8da6-473c-b738-1ab2721dd53b)


## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
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 forc good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: forc build panic when sway package folder contains dot Halimao/sway
2 participants
@xunilrj @ironcev