Bug 24357 - ModuleUpdater should set file permissions on Unix
ModuleUpdater should set file permissions on Unix
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Autoupdate
3.x
All Linux
: P2 (vote)
: 6.x
Assigned To: dlipin
issues@platform
:
: 150524 (view as bug list)
Depends on:
Blocks: 108252
  Show dependency treegraph
 
Reported: 2002-06-03 20:55 UTC by Jesse Glick
Modified: 2012-04-25 10:00 UTC (History)
7 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
patch that sets the file executable bits on Unix in ModuleUpdater (9.70 KB, patch)
2009-03-11 11:30 UTC, dlipin
Details | Diff
ModuleUpdater fix, org-yourorghere-executable-permissions.nbm and ExecutablePermissionsTest.java (20.31 KB, patch)
2009-03-11 16:41 UTC, dlipin
Details | Diff
Using Boolean.TYPE in setExecutable method retrieval, identify "universal binary" and "shell script" as the executables (20.74 KB, patch)
2009-03-11 19:30 UTC, dlipin
Details | Diff
ModuleUpdater reads Info/executable.list and chmods the listed files appropriately (4.71 KB, patch)
2009-03-13 13:43 UTC, dlipin
Details | Diff
patch to enable setting executable permissions on module build and nbm installation (31.45 KB, patch)
2009-08-07 12:19 UTC, dlipin
Details | Diff
hg bundle file with base rev#c4c83e0801de (9.68 KB, application/octet-stream)
2009-08-07 12:22 UTC, dlipin
Details
patch including fixes due to Jesse`s review (25.92 KB, patch)
2009-08-08 18:52 UTC, dlipin
Details | Diff
hg bundle with fixes due to Jesse`s review (8.45 KB, application/octet-stream)
2009-08-08 18:53 UTC, dlipin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2002-06-03 20:55:22 UTC
ModuleUpdater.unpack ought to preserve file
permissions. E.g. if you already have a file
$nbinstall/bin/runfoo.sh which is executable on
Unix, and you unpack foo.nbm containing
netbeans/bin/runfoo.sh in global mode,
$nbinstall/bin/runfoo.sh should continue to have
execute permissions.

Since foo.nbm might be a new module, I would
rather recommend either of these:

1. AU on Unix automatically does a chmod a+x
${file} for every file matching well-known names
such as *.sh.

2. The NBM XML DTD be augmented to include file
permission information, at least the execute bit,
and AU on Unix pay attention to it.
Comment 1 Jesse Glick 2002-06-04 00:13:00 UTC
In fact on Linux at least it does seem to work for updates: the old
permissions are kept.

Leaving open for the case of new modules.
Comment 2 Marek Grummich 2002-07-22 08:26:06 UTC
Target milestone was changed from '3.4' to TBD.
Comment 3 Marian Mirilovic 2002-12-06 18:50:39 UTC
reassigne to Hrebejk, new owner of autupdate 
Comment 4 Marian Mirilovic 2003-12-01 10:49:06 UTC
reassigne to Jirka - new owner of autoupdate
Comment 5 Jiri Rechtacek 2008-10-16 17:12:22 UTC
It's more likely a task on building NBMs to use smarter zip utility to hold file permission in NBM archive. I guess then
Plugin Manager could take care to hold as same permission on unpacked files from archive.
Comment 6 Jesse Glick 2008-10-17 23:08:52 UTC
*** Issue 150524 has been marked as a duplicate of this issue. ***
Comment 7 Jiri Rechtacek 2008-10-31 15:25:16 UTC
Quoted from mail discussion:
Jesse Glick wrote:
> JDK 6 has direct support for querying and setting basic file permission 
> bits in java.io.File. In JDK 5, this can be done only be forking a Unix 
> executable or using e.g. JNA.
> 
> Ant's <zip> implementation (written from scratch) is capable of storing 
> file permissions upon request. AFAIK it makes no attempt to pick up such 
> permissions automatically from files it is actually packing (and this 
> would anyway be rather difficult prior to JDK 6); rather, you can 
> specify a file mode in the build script for certain ZIP entries.
> 
> NB's <makenbm> uses Ant's ZIP implementation internally, so could be 
> modified somehow to include file mode bits based on some information.
> 
> AU's updater.jar uses java.util.zip, which has no representation of file 
> mode bits. If you wanted to set +x when unpacking NBMs, you would need 
> to somehow identify which files required this bit. Possibilities 
> include: (a) using a different ZIP implementation which can report on 
> the bits in the ZIP file; (b) requiring such files to be enumerated 
> somewhere in Info.xml or elsewhere in the NBM; (c) detecting likely 
> executables, e.g. any *.sh, or any file starting with "#!/", or any file 
> starting with an ELF magic header, etc Still pretty easy in JDK 5:
> just Runtime.exec /bin/chmod.
> 
> NBMs are packed from the cluster directory; default build targets in 
> common.xml etc. never set executable bits on files copied to this dir, 
> but build.xml overrides can be made to do so for specific cases.
Comment 8 Antonin Nebuzelsky 2008-11-14 15:32:07 UTC
Reassigning to the new "autoupdate/*" owner dlipin.
Comment 9 dlipin 2009-03-11 11:29:55 UTC
The proposed fix for option (c) is attached.
It does the following for all extracted files :
1) run /usr/bin/file and checkes whether the file is reported as "executable", "shared object", "shared library" or 
"dynamic lib"
2) checks if the file has the ELF magic header or the interpreter string (#!/)

At the end, it executes either /bin/chmod or /usr/bin/chmod on all the files at once (if running JDK5) or invokes 
java.io.File.setExecutable(true, false) if it is available (JDK6 and later).
Comment 10 dlipin 2009-03-11 11:30:58 UTC
Created attachment 78032 [details]
patch that sets the file executable bits on Unix in ModuleUpdater
Comment 11 Jaroslav Tulach 2009-03-11 12:22:07 UTC
Out of curiosity: Should not your write a unit test to verify that the intended behaviour works forever?
Comment 12 dlipin 2009-03-11 13:44:09 UTC
likely yes, but unfortunately there is no tests for the o.n.updater at all (except simple test on UpdaterDispatcher).
There is likely a reason for such situation but I don`t know it.
Comment 13 dlipin 2009-03-11 13:50:31 UTC
I guess that it can be tested via tests in o.n.m.autoupdate.services, I`ll write a new one soon.
Comment 14 Jaroslav Tulach 2009-03-11 16:19:30 UTC
Almost every test for autoupdate.services needs to deal with updater. Just modify or add a bit of code to one of the 
existing and test that permissions are OK. Imho.
Comment 15 dlipin 2009-03-11 16:39:50 UTC
Attaching the fix and test for it.
Comment 16 dlipin 2009-03-11 16:41:05 UTC
Created attachment 78060 [details]
ModuleUpdater fix, org-yourorghere-executable-permissions.nbm and ExecutablePermissionsTest.java
Comment 17 Jesse Glick 2009-03-11 18:33:42 UTC
Does it work on Mac OS X as well as Linux and Solaris? Is /usr/bin/file universally available and does it have similar
output?

BTW varargs:

  File.class.getMethod("setExecutable", new Class[] {Boolean.class, Boolean.class})

can be simplified to

  File.class.getMethod("setExecutable", Boolean.class, Boolean.class)

Anyway I expect that you meant the primitive Boolean.TYPE, not Boolean.class which represents the wrapper class.
Comment 18 dlipin 2009-03-11 19:09:36 UTC
> Does it work on Mac OS X as well as Linux and Solaris? 
> Is /usr/bin/file universally available and does it have similar output?
I`ve checked Linux (FC4, Ubuntu 8.10), Solaris 10 (sparc,x86), AIX 5.3, Mac OS X 10.4 and /usr/bin/file is available on 
all of them. I have also OpenBSD and FreeBSD - I`ll check them after unzip the old VBox images that I`ve created 
recently. Unfortunately I have no access to HP-UX. And the rest of systems is almost nothing among NB users.

Yes, the output pretty much the same. It is the one-line output with colon as the separator between the file path and 
file "type". Certainly, the number of supported files that /usr/bin/file can correctly identify depends on the system.

The only exception is the Mac OS X with its universal binaries. In that case the output of file is 4 lines long - but 
it can also be identified by "universal binary" substring.
$ file  /Applications/Chess.app/Contents/MacOS/Chess
/Applications/Chess.app/Contents/MacOS/Chess: Mach-O universal binary with 4 architectures
/Applications/Chess.app/Contents/MacOS/Chess (for architecture ppc):    Mach-O executable ppc
/Applications/Chess.app/Contents/MacOS/Chess (for architecture ppc64):  Mach-O 64-bit executable ppc64
/Applications/Chess.app/Contents/MacOS/Chess (for architecture i386):   Mach-O executable i386
/Applications/Chess.app/Contents/MacOS/Chess (for architecture x86_64): Mach-O 64-bit executable x86_64


> BTW varargs:
> 
>  File.class.getMethod("setExecutable", new Class[] {Boolean.class, Boolean.class})
>
> can be simplified to
>
>  File.class.getMethod("setExecutable", Boolean.class, Boolean.class)
> 
> Anyway I expect that you meant the primitive Boolean.TYPE, not Boolean.class which represents the wrapper class.
Sure, thanks, Boolean.TYPE is the right thing.
Comment 19 dlipin 2009-03-11 19:30:22 UTC
Created attachment 78070 [details]
Using Boolean.TYPE in setExecutable method retrieval, identify "universal binary" and "shell script" as the executables
Comment 20 dlipin 2009-03-11 19:58:34 UTC
I can also try to play with option (b) in the following way:
1) introduce property e.g. "executable.files" in nbproject/project.properties
2) during making NBM (MakeNBM) read that "executable.files" property and process it as the <includes> ant type i.e. 
masks are supported. All file paths store in Info/executables.list.
3) During building of the NB plugin project and copying all the files to nbbuild/netbeans (who does that? 
apisupport.harness? which target? "release"?) also read that "executable.files" property and after that do the chmod - 
it will help to avoid doing <chmod> in build.xml in the particular modules (e.g. cnd/build.xml, o.jruby.distro/
build.xml and such).
4) During NBM installation read that Info/executable.list and fix the permissions.
Comment 21 dlipin 2009-03-13 13:41:49 UTC
Attaching the other try to fix this issue according to 4-steps changes listed in the previous comment.
This diff makes the point (4).
Comment 22 dlipin 2009-03-13 13:43:08 UTC
Created attachment 78143 [details]
ModuleUpdater reads Info/executable.list and chmods the listed files appropriately
Comment 23 dlipin 2009-08-07 12:16:45 UTC
Anyone interested please review the patch, it touches autoupdate.services, apisupport.harness, nbbuild. I`m going to 
integrate it early next week.

The solution is described in my comment from "Wed Mar 11 18:58:34 +0000 2009" with the only correction in the property 
name (nbm.executable.files).
Comment 24 dlipin 2009-08-07 12:19:19 UTC
Created attachment 85957 [details]
patch to enable setting executable permissions on module build and nbm installation
Comment 25 dlipin 2009-08-07 12:22:54 UTC
Created attachment 85959 [details]
hg bundle file with base rev#c4c83e0801de
Comment 26 Jesse Glick 2009-08-07 17:25:20 UTC
[JG01] nbm.executable.files=launchers/app.sh in apisupport.harness is unnecessary AFAIK; this file is not run in place,
it is packed into a ZIP by an Ant target (during which time it is a+x).


[JG02] Prefer comma-separated properties to space-separated properties. In this case probably either separator is
accepted, but documentation should recommend comma-separated.


[JG03] If you change taskdefs.properties you probably must also change bundled.tasks in project.properties. Test on an
external standalone module to check.


[JG04] I think BufferedReader would be a lot simpler and clearer in readExecutableFilesList. Not to mention that your
impl ignores encoding. Use UTF-8. Don't use String.trim() without a compelling reason.


[JG05] java.version is the runtime environment version. Probably you meant java.specification.version. Anyway it is
probably more straightforward to check for File.setExecutable, catching NoSuchMethodException and falling back to chmod.


[JG06] Keeping binary archives (such as NBMs) in test sources is poor style. Better to create the JAR from source
entries when the test is run. org.openide.util.test.TestFileUtils make this easy.


[JG07, to supersede JG03 and perhaps JG02] Do you really need MakeExecutableFilesList? I doubt it. Just

<chmod perm="a+x">
  <fileset dir="${cluster}" includes="${nbm.executable.files}"/>
</chmod>
...
<nbm ...>
  <executables dir="${cluster}" includes="${nbm.executable.files}"/>
</nbm>

should work.


[JG08] The chmod-executables target should be a dependency of the 'netbeans' target, not antcall'd from release. (Should
follow 'release' and 'netbeans-extra' in the dep list.)
Comment 27 dlipin 2009-08-07 18:22:09 UTC
Re JG01: I still think it is required if apisupport.harness (NetBeans Plugin Development) is installed from UC.
In that case the launchers/app.sh is required to be chmod`ed after installation, right? Or is it chmoded when the 
platform app is been build?

Re [JG02] Both comma and space separation is allowed. It is processed like ant`s "includes" attribute ("comma- or space-
separated list of patterns of files that must be included"). I`ll fix the documentation.

Re [JG03] We`ll do, thanks.


[JG04] I think BufferedReader would be a lot simpler and clearer in readExecutableFilesList. Not to mention that your
impl ignores encoding. Use UTF-8. Don't use String.trim() without a compelling reason.


Re [JG05] Sure, will check for File.setExecutable and fallback to chmod.

Re [JG06] Agree. We`ll do.


Re [JG07] Yes, I do think that it is required. In case nbm.executable.files is a pattern (like bin/**), then doing the 
chmod you suggested will set executable permissions on *all* files already available in ${cluster}/bin/ directory, not 
only the files that belong to the particular module. In other words, doing so can result in unnecessary executable 
permission setting. Even though it is does not sound critical now, but I think better to avoid that than to fix 
possible bugs.

Re [JG08] Thanks, will fix.
Comment 28 Jesse Glick 2009-08-07 19:06:00 UTC
JG01 - no, it is not a+x even in a regular build. As I said, it is used only when a platform app is built, so the
harness script takes care of it.


JG07 - just set the property to those files your module actually owns. I don't think setting it to "bin/" is necessary
in any situation.
Comment 29 dlipin 2009-08-08 18:51:06 UTC
Attaching the patch and bundle with all fixes due to Jesse`s comments except JG06 (postponed, we have a few nbms 
already in test sources - not critical for this issue, will work on it some time later hopefully).
Comment 30 dlipin 2009-08-08 18:52:06 UTC
Created attachment 85994 [details]
patch including fixes due to Jesse`s review
Comment 31 dlipin 2009-08-08 18:53:05 UTC
Created attachment 85995 [details]
hg bundle with fixes due to Jesse`s review
Comment 32 dlipin 2009-08-11 12:35:14 UTC
core-main #f2ec4ce487be
Comment 33 Jesse Glick 2009-08-11 23:22:36 UTC
As an addendum to JG07, all this:

<target name="-init-executables"
depends="-check-executables,-define-executables-reference-defined,-define-executables-reference-undefined"/>
<target name="-check-executables">
    <condition property="has.executables">
        <and>
            <isset property="nbm.executable.files"/>
            <not>
               <equals arg1="${nbm.executable.files}" arg2=""/>
            </not>
        </and>
    </condition>
</target>
<target name="-define-executables-reference-defined" if="has.executables">
    <fileset dir="${cluster}" id="module.executable.files" includes="${nbm.executable.files}"/>
</target>
<target name="-define-executables-reference-undefined" unless="has.executables">
    <fileset dir="${cluster}" id="module.executable.files" excludes="**"/>
</target>

can probably be replaced with just

<target name="-init-executables">
    <property name="module.executable.files" value="[NOTHING]"/>
    <fileset dir="${cluster}" id="module.executable.files" includes="${nbm.executable.files}"/>
</target>
Comment 34 Quality Engineering 2009-08-12 06:28:25 UTC
Integrated into 'main-golden', will be available in build *200908120201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f2ec4ce487be
User: Dmitry Lipin <dlipin@netbeans.org>
Log: Issue #24357 ModuleUpdater should set file permissions on Unix
Comment 35 dlipin 2009-08-18 12:22:16 UTC
#309e1ad6a73d, 1d736701d1bd
Comment 36 Quality Engineering 2009-08-21 06:10:05 UTC
Integrated into 'main-golden', will be available in build *200908210201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/1d736701d1bd
User: Dmitry Lipin <dlipin@netbeans.org>
Log: Fix property name (Issue #24357)
Comment 37 Vladimir Voskresensky 2009-08-24 22:27:31 UTC
I have tried to remove chmod from cnd/build.xml by replacing with nbm.executable.files
But it doesn't work, because chmod-executables in harness doesn't work (tried with -v):
chmod-executables:
Skipped because property 'has.executables' not set.
Comment 38 Jesse Glick 2009-08-24 22:49:59 UTC
dlipin probably forgot:

diff --git a/nbbuild/templates/common.xml b/nbbuild/templates/common.xml
--- a/nbbuild/templates/common.xml
+++ b/nbbuild/templates/common.xml
@@ -254,8 +254,7 @@
         </copy>        
     </target>
 
-    
-    <target name="chmod-executables" depends="-init-executables" if="has.executables">
+    <target name="chmod-executables" depends="-init-executables">
         <chmod perm="a+x" dir="${cluster}">
             <fileset refid="module.executable.files"/>
         </chmod>
Comment 39 Vladimir Voskresensky 2009-08-25 00:12:23 UTC
still doesn't work.
Has anybody tried it?

Info/executable.list is generated correctly, but during build chmod is not executed.

Btw, perm="a+x" could be not enough.
perm="a+rx" is better (to prevent umask based problems of access denied do to lack of read access)
Comment 40 Jesse Glick 2009-08-25 00:19:10 UTC
All files installed by AU should be readable to begin with, which is why only +x is necessary.
Comment 41 Vladimir Voskresensky 2009-08-25 03:05:05 UTC
agree, but target chmod-executables is used not by installer. it is used during netbeans build => umask of user can
affect the run of shared built netbeans by other users
Comment 42 dlipin 2009-08-25 12:54:02 UTC
Indeed, I forgot it. Fixed in core-main#e52b18db8a05. Thanks, Jesse!
Comment 43 dlipin 2009-08-25 13:01:07 UTC
Vladimir, if that fix still does not work for you, please attach the ant verbose (-v) log and the value of your 
nbm.executable.files property.
Comment 44 Vladimir Voskresensky 2009-08-26 01:15:15 UTC
I have
nbm.executable.files=bin/dorun.sh

It doesn't work.
....
bin/dorun.sh added as bin/dorun.sh doesn't exist.
bin/stdouterr.bat added as bin/stdouterr.bat doesn't exist.
bin/stdouterr.sh added as bin/stdouterr.sh doesn't exist.
No sources found.
Copying 3 files to /home/vv159170/devarea/trunk/nbbuild/netbeans/cnd2
Copying /home/vv159170/devarea/trunk/cnd/release/bin/stdouterr.sh to
/home/vv159170/devarea/trunk/nbbuild/netbeans/cnd2/bin/stdouterr.sh
Copying /home/vv159170/devarea/trunk/cnd/release/bin/dorun.sh to
/home/vv159170/devarea/trunk/nbbuild/netbeans/cnd2/bin/dorun.sh
Copying /home/vv159170/devarea/trunk/cnd/release/bin/stdouterr.bat to
/home/vv159170/devarea/trunk/nbbuild/netbeans/cnd2/bin/stdouterr.bat
release:
-init-executables:
Override ignored for property "nbm.executable.files"
chmod-executables:
Current OS is Linux
verify-class-linkage:
....
Comment 45 Quality Engineering 2009-08-26 06:40:39 UTC
Integrated into 'main-golden', will be available in build *200908260201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/e52b18db8a05
User: Dmitry Lipin <dlipin@netbeans.org>
Log: Issue #24357 ModuleUpdater should set file permissions on Unix
Comment 46 dlipin 2009-08-26 08:51:22 UTC
Should be fixed now in 
http://hg.netbeans.org/core-main/rev/c031e5372024
Sorry for inconvenience.
Comment 47 Quality Engineering 2009-08-26 17:52:25 UTC
Integrated into 'main-golden', will be available in build *200908261401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/c031e5372024
User: Dmitry Lipin <dlipin@netbeans.org>
Log: Issue #24357 ModuleUpdater should set file permissions on Unix
Comment 48 Quality Engineering 2012-04-25 10:00:04 UTC
Integrated into 'main-golden', will be available in build *201204250400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/7251aab540a1
User: Jesse Glick <jglick@netbeans.org>
Log: NetBeansInOSGi near-equivalent of #24357 (exec bits on files unpacked from NBM fileset).


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo