[2.0] dvc.yaml: doc isexec field

[efiop]

Per treeverse/dvc#5061

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

[efiop]

For the record: this is indeed for 2.0, but we have some plans to create a new dvc.lock schema for 2.0 too, so the location of isexec might adjust a bit, but that doesn't really affect this PR.

Also, not sure how deep to go explaining isexec and posix acl behind it.

[jorgeorpinel]

Note: I'm repurposing the migration label for now to mark 2.0 PRs and help me manage the v1 release branch vs master as soon as the first 2.0 PR gets merged.

[jorgeorpinel]

not sure how deep to go explaining isexec and posix acl

I see no need to go into much details at all. Will leave a suggestion with generic language ⌛

[jorgeorpinel]

Suggested change

	- `isexec`: If a file, whether or not owner has execute permission. On Windows,
	it is not saved and, if set manually, won't have any effect on `dvc checkout`
	and `dvc pull`.
	- `isexec`: Whether this is an executable file. DVC preserves execute
	permissions upon `dvc checkout` and `dvc pull`. This has no effect on
	directories, or in general on Windows.

(same for deps)

[jorgeorpinel]

Questions:

• Do we want to specify "preserves POSIX execute permissions"? (possibly linked)
  TBH I see no need since we say it doesn't work on Windows so it's easy to infer why.
• "upon dvc checkout and dvc pull" - are these the only commands it affects? What about add or commit (when tracking stuff which moves it to the cache and then links back @efiop?

Thanks

[efiop]

Do we want to specify "preserves POSIX execute permissions"? (possibly linked)
TBH I see no need since we say it doesn't work on Windows so it's easy to infer why.

Thought about it too, but I feel it is generally understood as is, so I wouldn't clarify it more.

"upon dvc checkout and dvc pull" - are these the only commands it affects? What about add or commit (when tracking stuff which moves it to the cache and then links back @efiop?

Pretty much yes. add/commit take existing file and add/commit it, so on windows there is nothing to capture - no exec bit to set. checkout/pull is when the exec bit is coming from another machine (e.g. your collegue on linux) or set by hand, and thats where it is better to clarify it.

[jorgeorpinel]

Hmm. I just tried this on Ubuntu (WSL) and DVC (1.11.8) removes the exec bit upon add. No reflinks on my system though so it's a copy, but the cached file doesn't have exec on either.

[efiop]

@jorgeorpinel 1.11.8 doesn't have that patch yet and it is expected for it to not preserve exc. You need to try upstream dvc.

[jorgeorpinel]

Sure but my question was: since add does remove the exec bit, will the exec field affect add after all? Thinking about it, that doesn't make sense as there is no .dvc file prior to add/commit... so I'm re-approving this.

But (unrelated to this PR) should there perhaps be a new --isexec flag on those or other commands (run/repro?) to do this? Moved to treeverse/dvc#4578 (comment)

[efiop]

@jorgeorpinel Do you mean a flag to set isexec value through CLI? Not really, there is no need.

[jorgeorpinel]

Oh I see you answered here. OK

Question pending in #2039 (comment)

[shcheklein]

hmm, sorry guys .. I'm way too late (and approving this PR) ... just to clarify - what does it mean for deps. I though pull/checkout do now do anything to deps?

How would it work if out say exec:True and dep says exec:False?

[efiop]

Oops, this was not meant to be here. Deleting.

[shcheklein]

btw, @jorgeorpinel migration label looks strange, we are not migrating anything as far as I know - we are releasing a new version.

[jorgeorpinel]

True. OK I made a new label and changed the title.

Let's not merge this yet? (because it won't be released until Jan per treeverse/dvc#4578 (comment))

[jorgeorpinel]

@shcheklein This won't be released until Jan. Is it OK to merge these? I added the 2.0 label in part to delay merging some of these PRs at least for some time.

[jorgeorpinel]

UPDATE: I see your #2031 (comment) that we should start tracking and backporting.