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

Added condition to skip lines starting with exclamation #2751

Merged
merged 3 commits into from
Jun 7, 2022

Conversation

teresaromero
Copy link
Member

Signed-off-by: Teresa Romero teresa@okteto.com

Fixes #2679

Proposed changes

  • Added the skip condition when parsing the .stignore file - when starts with ! do not prepend (?d)

Context

Using the repo from @AgustinRamiroDiaz https://github.com/AgustinRamiroDiaz/okteto-stignore i've been going around the different possibilities. The remote .stignore is being parsed from the local one, we add the prefix (?d) in order to be able to delete a directory. Lines regarding #includes or !<file or folder> where not copied to this remote .stignore, i thought this had to be copy to the remote file but when i did, syncing got in an infite loop whith the following error:

WARNING: Error on folder \"1\" (okteto-1): loading ignores: parse error: failed to load include file .numbers: open /okteto/.numbers: no such file or directory" process=syncthing

So decided to just avoid lines starting with ! as we do with #, as, synthing is using the local rules, although the remote rules are different.

Signed-off-by: Teresa Romero <teresa@okteto.com>
@teresaromero teresaromero requested a review from a team June 3, 2022 09:51
@@ -80,6 +80,9 @@ func addStignoreSecrets(dev *model.Dev) error {
if strings.HasPrefix(line, "#") {
continue
}
if strings.HasPrefix(line, "!") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add tests for this function?

@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #2751 (6278c84) into master (a45a14d) will increase coverage by 0.19%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #2751      +/-   ##
==========================================
+ Coverage   31.84%   32.03%   +0.19%     
==========================================
  Files         158      158              
  Lines       18553    18555       +2     
==========================================
+ Hits         5908     5945      +37     
+ Misses      11945    11905      -40     
- Partials      700      705       +5     
Impacted Files Coverage Δ
cmd/up/stignore.go 31.62% <75.00%> (+31.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a45a14d...6278c84. Read the comment docs.

@jLopezbarb
Copy link
Contributor

What about lines starting with #include. The user said that it was failing to include from other files. Is it working?

@teresaromero
Copy link
Member Author

What about lines starting with #include. The user said that it was failing to include from other files. Is it working?

Using the repo of agusting, i was including .numbers and also in my local a .gitignore, and it was working on my remote dev, the ignored rules where working even though the .stignore did not reflect the #include lines

Signed-off-by: Teresa Romero <teresa@okteto.com>
Signed-off-by: Teresa Romero <teresa@okteto.com>
@teresaromero teresaromero merged commit 6882dc7 into master Jun 7, 2022
@teresaromero teresaromero deleted the tere/2679-fix-stignore-patterns branch June 7, 2022 09:20
@llindemann
Copy link

llindemann commented Jun 18, 2022

since this change, the following local .stignore file doesn't work for me:

!/src/
!/public/

*

pre 2.4.0 /src and /public were synced and everything else ignored

@ifbyol
Copy link
Member

ifbyol commented Jun 27, 2022

@llindemann We are working on a fix we'll let you know when it is released

@teresaromero
Copy link
Member Author

@llindemann a fix was released with 2.4.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird behavior with .stignore patterns
7 participants