public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Eugene Loh <eugene.loh@oracle.com>
Cc: Kris Van Hees <kris.van.hees@oracle.com>,
	dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH 2/4] test: correct file permissions
Date: Thu, 18 Dec 2025 15:17:51 -0500	[thread overview]
Message-ID: <aURhb26qwDFCqmKk@oracle.com> (raw)
In-Reply-To: <afcf46e3-ed97-8bd3-7bb1-8268df5660cf@oracle.com>

On Thu, Dec 18, 2025 at 03:07:29PM -0500, Eugene Loh wrote:
> I'd like some context here.  Why are these permissions being changed?  E.g.,
> 
> * There are some *.d files that start with "#!" but they're invoked as D
> scripts, not interpreter files.  So, they don't need +x.

Well, yes, they do not need +x but then they ALSO do not need the shebang.
(See below)

> * There are some *.r files given +x.  Why?  They aren't being executed, are
> they?

Some tests (I believe some you wrote, actually) generate a shell script, with
shebang, and expect it to be compared to a .r file.  I presume that the intent
is that such files are expected to be executed, and therefore they should bear
the x permission.  If they are not expected to be executed, then I think it is
best that the test does not generate a shebang shell script at all.

> * A D script is turned -x.  Okay, but not consequential?

I agree that some of the file permission changes may not be correct - I did
it semi-automatically based on shebang presence when in reality a lot of those
trsts probably should not have the shebang.  I can go through the list and do
a more thorough job at this.  I admit this was primarily to get lintian to
stop complaining.  It is very usrful to flag potential issues, but the fix I
did for various cases certainly may not be the ideal one.

> I'm not against the patch.  I just don't understand it.  There's more to x
> permissions than whether there is a leading shebang.

I agree - when I included this patch in the series I forgot I had not done a
full review of the changes to determine whether they were the best course of
action for each case.  As mentioned above, shebang on D scripts that are passed
to dtrace is pointless and should be removed.  Except perhaps for having one
test that ensures that shebang presence doesn't cause dtrace to choke.  That
can be a singular exception.

> On 12/17/25 00:09, Kris Van Hees wrote:
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   test/demo/script/interp.d                                         | 0
> >   test/demo/script/tracewrite.d                                     | 0
> >   test/demo/spec/specopen.d                                         | 0
> >   test/stress/fbtsafety/tst.vahole.d                                | 0
> >   test/unittest/funcs/substr/tst.substr.r                           | 0
> >   test/unittest/funcs/tst.basename.r                                | 0
> >   test/unittest/funcs/tst.index.r                                   | 0
> >   test/unittest/lockstat/tst.lockstat-summary.d                     | 0
> >   test/unittest/scripting/err.D_MACRO_UNDEF.invalidargs.d           | 0
> >   test/unittest/scripting/err.D_OP_LVAL.rdonly.d                    | 0
> >   test/unittest/scripting/err.D_OP_WRITE.usepidmacro.d              | 0
> >   test/unittest/scripting/err.D_SYNTAX.concat.d                     | 0
> >   test/unittest/scripting/err.D_SYNTAX.desc.d                       | 0
> >   test/unittest/scripting/err.D_SYNTAX.inval.d                      | 0
> >   test/unittest/scripting/err.D_SYNTAX.pid.d                        | 0
> >   test/unittest/scripting/tst.arg0.d                                | 0
> >   test/unittest/scripting/tst.assign.d                              | 0
> >   test/unittest/scripting/tst.basic.d                               | 0
> >   test/unittest/scripting/tst.pgid.d                                | 0
> >   test/unittest/scripting/tst.pid.d                                 | 0
> >   test/unittest/scripting/tst.sid.d                                 | 0
> >   test/unittest/scripting/tst.trace.d                               | 0
> >   .../unittest/speculation/err.D_ACT_SPEC.SpeculateWithBreakPoint.d | 0
> >   test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithChill.d     | 0
> >   test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d   | 0
> >   .../unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d | 0
> >   test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithPanic.d     | 0
> >   test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithRaise.d     | 0
> >   test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithStop.d      | 0
> >   29 files changed, 0 insertions(+), 0 deletions(-)
> >   mode change 100644 => 100755 test/demo/script/interp.d
> >   mode change 100644 => 100755 test/demo/script/tracewrite.d
> >   mode change 100644 => 100755 test/demo/spec/specopen.d
> >   mode change 100644 => 100755 test/stress/fbtsafety/tst.vahole.d
> >   mode change 100644 => 100755 test/unittest/funcs/substr/tst.substr.r
> >   mode change 100644 => 100755 test/unittest/funcs/tst.basename.r
> >   mode change 100644 => 100755 test/unittest/funcs/tst.index.r
> >   mode change 100755 => 100644 test/unittest/lockstat/tst.lockstat-summary.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_MACRO_UNDEF.invalidargs.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_OP_LVAL.rdonly.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_OP_WRITE.usepidmacro.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_SYNTAX.concat.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_SYNTAX.desc.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_SYNTAX.inval.d
> >   mode change 100644 => 100755 test/unittest/scripting/err.D_SYNTAX.pid.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.arg0.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.assign.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.basic.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.pgid.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.pid.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.sid.d
> >   mode change 100644 => 100755 test/unittest/scripting/tst.trace.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithBreakPoint.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithChill.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithPanic.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithRaise.d
> >   mode change 100644 => 100755 test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithStop.d
> > 
> > diff --git a/test/demo/script/interp.d b/test/demo/script/interp.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/demo/script/tracewrite.d b/test/demo/script/tracewrite.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/demo/spec/specopen.d b/test/demo/spec/specopen.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/stress/fbtsafety/tst.vahole.d b/test/stress/fbtsafety/tst.vahole.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/funcs/substr/tst.substr.r b/test/unittest/funcs/substr/tst.substr.r
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/funcs/tst.basename.r b/test/unittest/funcs/tst.basename.r
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/funcs/tst.index.r b/test/unittest/funcs/tst.index.r
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/lockstat/tst.lockstat-summary.d b/test/unittest/lockstat/tst.lockstat-summary.d
> > old mode 100755
> > new mode 100644
> > diff --git a/test/unittest/scripting/err.D_MACRO_UNDEF.invalidargs.d b/test/unittest/scripting/err.D_MACRO_UNDEF.invalidargs.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_OP_LVAL.rdonly.d b/test/unittest/scripting/err.D_OP_LVAL.rdonly.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_OP_WRITE.usepidmacro.d b/test/unittest/scripting/err.D_OP_WRITE.usepidmacro.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_SYNTAX.concat.d b/test/unittest/scripting/err.D_SYNTAX.concat.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_SYNTAX.desc.d b/test/unittest/scripting/err.D_SYNTAX.desc.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_SYNTAX.inval.d b/test/unittest/scripting/err.D_SYNTAX.inval.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/err.D_SYNTAX.pid.d b/test/unittest/scripting/err.D_SYNTAX.pid.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.arg0.d b/test/unittest/scripting/tst.arg0.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.assign.d b/test/unittest/scripting/tst.assign.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.basic.d b/test/unittest/scripting/tst.basic.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.pgid.d b/test/unittest/scripting/tst.pgid.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.pid.d b/test/unittest/scripting/tst.pid.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.sid.d b/test/unittest/scripting/tst.sid.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/scripting/tst.trace.d b/test/unittest/scripting/tst.trace.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithBreakPoint.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithBreakPoint.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithChill.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithChill.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithPanic.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithPanic.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithRaise.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithRaise.d
> > old mode 100644
> > new mode 100755
> > diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithStop.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithStop.d
> > old mode 100644
> > new mode 100755

  reply	other threads:[~2025-12-18 20:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17  5:09 [PATCH 2/4] test: correct file permissions Kris Van Hees
2025-12-18 20:07 ` Eugene Loh
2025-12-18 20:17   ` Kris Van Hees [this message]
2025-12-18 21:54     ` Eugene Loh
2025-12-18 22:09       ` Kris Van Hees
2025-12-19  0:11         ` Eugene Loh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aURhb26qwDFCqmKk@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox