Closed GoogleCodeExporter closed 9 years ago
Original comment by richard.eckart
on 7 Jan 2013 at 4:21
I think using PWD is a bad idea as that may be basically anything, but trying
DKPRO_HOME and the user's home should work.
Original comment by richard.eckart
on 7 Jan 2013 at 4:22
At least on *nix systems, executable files should never be put in the system
temp directory by default, nor should they be put in some arbitrary location in
the user's home directory. Instead we should follow the conventions of the XDG
Base Directory Specification
<http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html> –
that is, check for the presence of an $XDG_RUNTIME_DIR environment variable,
and if set, use that directory for the binary. On systems implementing the XDG
Base Directory Specification (which is pretty much every modern *nix), this
directory is guaranteed to be readable, writable, and executable by the user
(and only the user). If this environment variable is missing or empty, only
then should the application try falling back to the system temp directories.
On POSIX/FHS systems, this is $TMPDIR (or failing that, /tmp). Microsoft
Windows probably has its own conventions but I don't know what those are.
Original comment by tristan.miller@nothingisreal.com
on 9 Jan 2013 at 11:16
An excellent idea. Thanks for pointing this out.
Original comment by richard.eckart
on 9 Jan 2013 at 1:41
Original comment by richard.eckart
on 16 Feb 2013 at 11:01
Original comment by pedrobss...@gmail.com
on 26 Apr 2013 at 1:00
/tmp directory is not used any more. Now it is checked if the
XDG_RUNTIME_DIR variable is set. If it is, this folder is used. Otherwise, it is
checked if the DKPRO_HOME environment is set. If it is, then it is used. If it
is not, then the USER.HOME folder is used.
Original comment by pedrobss...@gmail.com
on 26 Apr 2013 at 3:49
I think it's not quite fixed yet. E.g. there is no handling of DKPRO_HOME (or
any of the other variables) being set but not having been created as a
directory yet. The getClasspathAsFolder() isn't updated yet.
Original comment by richard.eckart
on 26 Apr 2013 at 3:55
I tested it without having the directory created, and it created on the fly. At
least on ubuntu is not a problem. Do you know if it is problem in other OS? The
getClasspathAsFolder() method was updated, thanks for noticing this.
Original comment by pedrobss...@gmail.com
on 26 Apr 2013 at 5:05
Since we do not use /tmp anymore at all now, we get test failures on the build
server, because the home folder is actually not writable., e.g. in
testGetUrlAsFile().
We also seem to have a problem that the temporary files created by getUrlAsFile
can actually conflict with each other. E.g. if
getUrlAsFile("http://lala/test.txt") and then
getUrlAsFile("http://lolo/test.txt") is called, the temporary file created in
the first call appears to be overwritten.
Original comment by richard.eckart
on 26 Apr 2013 at 6:09
The problem of conflicting file names was not there previously, because
File.createTempFile(name, "."+suffix) creates unique names. The ability to
create a unique name for each call of createTempFile() must be retained.
Original comment by richard.eckart
on 26 Apr 2013 at 6:14
I think your test did not what you think it did. I just ran a manual test
pointing XDG_RUNTIME_DIR to a folder that does not exist and ran the
testGetUrlAsFile(). The result was a FileNotFoundException because
FileOutputStream could not write a file to a non-existing folder.
So we have several problems now:
- /tmp is not used anymore at all, even if it could be used
- createTempFile() does not create unique names like File.createTempFile() does
- the case that the temporary directory does not exist is not handled
- also the case that a temporary directory does exist but is not writable that
it cannot be used for executable files is not handled
I'd suggest the following:
- revert getUrlAsFile() and getClasspathAsFolder() to the previous state and
document that they must not be used for executable binaries
- create a new getUrlAsExecutable() method which makes sure that the temporary
file is created in a location which permits executables
- getUrlAsExecutable() should try to create the executable file in the methods
and order:
-- File.createTempFile() then try to make temporary file executable
-- if this fails try creating the executable in XDG_RUNTIME_DIR under a unique
name. Make sure no file which by accident may already have that name in that
location is overwritten
-- if this fails, try DKPRO_HOME, but according to policy not DKPRO_HOME
directly a subdirectory, I suggest DKPRO_HOME/temp. Again be careful about the
unique name and about not overwriting
-- if this fails, try the users' home. Again not the home directly, but
user.home/.dkpro/temp. (unique name/overwriting/yaddayadda)
-- if this also fails, produce an IOException with a useful error message
explaining what was tried, what failed, why and what is suggested to fix the
situation (e.g. make sure that user.home/dkpro/temp is writable and can be used
for executables)
Making sure that the created file really has a unique name component may the
the most difficult part. Apparrently the JDK uses a SecureRandom.nextLong() for
this. Not fool-proof, but probably good enough for our purposes.
Original comment by richard.eckart
on 26 Apr 2013 at 6:43
It's probably also a good idea to use
FileSystem.getFileSystem().createFileExcusively(file) to create the temporary
file, like File.createTempFile() does. Best take a look at the sources of
File.createTempFile() but don't just copy it ;)
Original comment by richard.eckart
on 26 Apr 2013 at 6:49
Apparrently FileSystem is an internal class of the JDK that we cannot use, so
we'll probably have to resort to a simple loop which repeatedly tests if there
already is a file with the unique name, try the next unique name if there is
and tries to create the file if it's not there. Then we pray that the situation
that two threads or processes try to create a file by the same name in almost
the same moment is so rare that it is no problem.
Original comment by richard.eckart
on 26 Apr 2013 at 7:09
Original comment by pedrobss...@gmail.com
on 3 May 2013 at 1:48
I'm having trouble following what the problem is here. You say there's a
possibility of conflicting file names and in Comment #14 propose some sort of
polling loop as a workaround. What's wrong with just using
File.createTempFile(String prefix, String suffix, File directory)?
Original comment by tristan.miller@nothingisreal.com
on 6 May 2013 at 10:50
Hum, inability to read the documentation on my side I guess ;) I didn't know of
that signature.
Original comment by richard.eckart
on 6 May 2013 at 10:52
OK. As a follow-up question, though, how do you intend to make the file
executable? DKPro uses Java 6, but support for changing POSIX file permissions
first appears in Java 7.
Original comment by tristan.miller@nothingisreal.com
on 6 May 2013 at 2:26
If Java 7, then we can use setExecutable(). Otherwise, there are some
provisions in the PlatformDetector class to help, in particular getChmodCmd().
I don't know if this is used anywhere right now.
Original comment by richard.eckart
on 6 May 2013 at 3:38
Actually, Java 6 already supports File.setExecutable(true). The provisions in
PlatformDetector had been used when the stuff was still supporting Java 5.
Original comment by richard.eckart
on 6 May 2013 at 3:43
Original comment by pedrobss...@gmail.com
on 8 May 2013 at 8:20
Original issue reported on code.google.com by
nohtae...@gmail.com
on 7 Jan 2013 at 10:53