winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.68k stars 181 forks source link

Issue with GitKraken related to projen readonly files #321

Closed 3p3r closed 1 year ago

3p3r commented 1 year ago

Can we PLEASE not do this? Having projen files marked as readonly is:

1- Completely unnecessary and annoying from a design point of view. 2- Completely screws up GitKraken and GitLens in vscode.

Can we PLEASE not do this?

If something's not supposed to change, it'll be caught in PRs.

Thanks!

3p3r commented 1 year ago

git clean also barfs up with these readonly files present.

3p3r commented 1 year ago

Also can't change branches in Gitkraken anymore because readonly files break Gitkraken's "auto-stash before changing branches" behavior.

Chriscbr commented 1 year ago

I want to preface with saying I'm totally fine with us reconfiguring how we're using projen in our monorepo (or even dropping it if it's causing too many problems).

But before we discuss solutions. could you give a bit more detail about the issue you were running into? eg how would one reproduce it?

Context: projen managed files are marked readonly as a guardrail so that users or other build tools don't accidentally modify them directly (since any changes would get overridden the next time projen is re-run). I think the git CLI handles readonly files without issue, but maybe there are some cases where it doesn't work well? I'd like to understand this better

3p3r commented 1 year ago

Setup a projen repo with VSCode+GitLens+GitKraken and nothing works correctly. Going from a branch to another branch where projen files are different? GitKraken dies. Switching branches? GitKraken dies mid-way and leaves you with an unclean branch switch (auto-stash fails). Running git clean -xdf dies as well because it can't touch readonly files if they're not checked in.

On that context note, projen needs to make a lockfile and track file hashes instead of being intrusive into user files. I don't particularly care if a file is managed by projen or not. I just need something like projen doctor or a similar command to tell me if I still have the same files projen expects. And running projen again overwrites them anyway.

eladb commented 1 year ago

I am not sure I fully understand why GitKraken or git clean die because of readonly files. I am not using GitKraken, but never had any problems with git clean.

Maybe a little screen recording can help us understand?

@3p3r, I would also recommend to open an issue about this in the projen project? I think it's worth a deeper dive and discussion at that level because it's likely something that others might encounter and it makes more sense to solve systemically.

MarkMcCulloh commented 1 year ago

On that context note, projen needs to make a lockfile and track file hashes instead of being intrusive into user files.

And running projen again overwrites them anyway.

I'm pretty sure projen doesn't just blindly replace files anymore, it should only write to disk if changes are needed. So if this is happening to you I would guess it's a projen bug or something unexpected about your setup.

In any case, I also would agree with the change. I always thought that projen should take more responsibility in detecting unexpected changes rather than the file perm approach which is very unlike most generative devtools.

eladb commented 1 year ago

projen should take more responsibility in detecting unexpected changes

Projen has an anti-tamper feature that runs during CI and detects changes to files in addition to the read-only attribute. The read-only attribute is just a way to signal to developers that the file should not be updated at an early stage. This is the idea of the readonly attribute.

eladb commented 1 year ago

Updated title to reflect the pain.

3p3r commented 1 year ago

@eladb These videos show a branch switch in GitKraken that dies mid-way during auto-stashing. GitKraken fails because it can't touch those readonly files. It needs to stash them (remove them) to prep for a branch switch. Almost all Git GUIs do this.

You can also see a subsequent git clean -xdf that actually does not do what it normally does, because it can't touch those readonly files.

The readonly attribute is part of the file content. It should not be messed with by a tool. It's no better than inserting a random line somewhere in the text.

https://user-images.githubusercontent.com/5657848/197824038-a707234a-2ee6-4be0-897e-16ca30df7c54.mp4

https://user-images.githubusercontent.com/5657848/197824592-04a50fc2-cbe8-42a5-95be-9a6df212ee1d.mp4

3p3r commented 1 year ago

If it's not clear in the video, I went from the main branch to my branch, it died mid way and left me with an unclean workspace. The branch is still main and running a git clean -xdf to get rid of the extra files does not do anything. If I forcefully remove the readonly attribute from all files and redo this, it works as expected.

eladb commented 1 year ago

Are these files also read only in your branch or are they read-write?

3p3r commented 1 year ago

they are readonly in both branches. both branches are just a few commits apart. they don't have a huge diff.

3p3r commented 1 year ago

Got another instance: my branch is one merge commit behind main and merge fails because projen's deps.json is readonly.

https://user-images.githubusercontent.com/5657848/197884493-b1c2e0b1-4ba2-4869-9217-a9be57622be2.mp4

On the CLI git merge main also fails unless I chmod everything read write.

eladb commented 1 year ago

Something is weird.... I've never had any issues with these files... This needs further investigation. Could be a Windows related issue?

On Wed, Oct 26, 2022 at 12:25 AM Sepehr Laal @.***> wrote:

Got another instance: my branch is one merge commit behind main and merge fails because projen's deps.json is readonly.

https://user-images.githubusercontent.com/5657848/197884493-b1c2e0b1-4ba2-4869-9217-a9be57622be2.mp4

On the CLI git merge main also fails unless I chmod everything read write.

— Reply to this email directly, view it on GitHub https://github.com/monadahq/winglang/issues/321#issuecomment-1291160969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESGDHR6RTPEXZJ6X6AXDDWFBF2XANCNFSM6AAAAAARNIR4GQ . You are receiving this because you were mentioned.Message ID: @.***>

3p3r commented 1 year ago

This is in WSL2 with Ubuntu 22.04.

3p3r commented 1 year ago

In the meantime while we investigate, can we disable this behavior? I don't know enough about projen and I am afraid of touching something that breaks a whole lot of other things.

eladb commented 1 year ago

@3p3r can you please open an issue with projen? curious what other people in the projen community think

3p3r commented 1 year ago

@eladb absolutely. will be happy to help. I need to setup a repro case though. Hesitant to post videos above which is captured over private repos opened in gitkraken.

github-actions[bot] commented 1 year ago

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!