-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
(Slightly) expand executable check to include os.stat #290
Conversation
Use of virtualized platforms is increasing in popularity. This includes using them to create consistent environments for local development, despite underlying platforms potentially varying widely. Even within a single organization of non-trivial size, it's not unusual to have developers running several different versions of a single OS on several different hardware revs from the same manufacturer. Containerization is a popular way to attempt to eliminate environmental differences to achieve consistency. Some virtualization layers confuse permissions checks inside virtualized environments when those environments have access to host filesystems. docker/for-mac#5509 is a prime example. This expands the ways in which file executability is detected slightly. The prior os.access check remains. If that should fail, this change includes an os.stat fallback. Together, these should get closer to detecting whether the file is truly executable on systems where such confusion is present. Fixes 289.
@asottile, I understand you don't want to accept this, but can you offer some insight as to why? |
there's several duplicates which answer that -- please search the issue tracker next time |
I did search. I thought I made a case for why your response to #189 should be revisited, and offered, what I thought were novel arguments in favor of accommodating your users with minimal intrusion into code integrity. I accept those may not have been persuasive, but I have no insight into your reasoning. You closed the issue and this PR so quickly, that I'm not even sure my points were considered? |
I hope you understand that questioning past decisions for revisiting without providing new arguments is a waste of everyones time. please do not do that |
I'm a bit confused about this. From my perspective, my arguments were novel, as was my proposed solution. If you look at my proposed diff and compare it to that which was rejected in #189, you'll see it's different. I attempted to walk a line between not offering a solution to users (your default position) and something less of a deviation than #189 that preserved original behavior. I also presented a case why I thought such a change was a good idea and why it went beyond merely working around a Docker bug, which hadn't been discussed before. I think you closed each of this PR and the corresponding issue within like 60 seconds of their respective filings. Maybe they weren't novel after all, or maybe I didn't make the differences or nuances I presented accessible, but providing something new was my understanding of what I was doing. The whole point in offering this solution is to save your users' time in a world where they are increasingly likely to encounter the problem, so I don't think revisiting this issue is "wasting" anyone's time. If anything, I'm trying to provide a means for your users to not waste their time by encountering this friction, scratching their heads, and spending their afternoon googling, maybe landing on this, #289, or #189, reading to understand what the problem is, then finding that they have no solution to it. If saving developer time is your goal, then maybe that's an additional argument that should be considered with this PR? Or maybe your customers' time isn't what you mean by "everyone's"? I'm unclear. |
the decision in the duplicate is "we're not hacking around a mistake made by docker inc" |
Yes, and the argument that I presented in #289 is that calling it a "mistake" is reductive and not accurate. It's a deliberate tradeoff based on other limitations, like FUSE, and other OS limitations. But, hey, it's your show. |
Use of virtualized platforms is increasing in popularity. This includes using them to
create consistent environments for local development, despite underlying platforms
potentially varying widely. Even within a single organization of non-trivial size, it's
not unusual to have developers running several different versions of a single OS on
several different hardware revs from the same manufacturer. Containerization is a
popular way to attempt to eliminate environmental differences to achieve consistency.
Some virtualization layers confuse permissions checks inside virtualized environments
when those environments have access to host filesystems. docker/for-mac#5509 is a prime
example.
This expands the ways in which file executability is detected slightly. The prior
os.access check remains. If that should fail, this change includes an os.stat fallback.
Together, these should get closer to detecting whether the file is truly executable on
systems where such confusion is present.
Fixes #289.