Return-Path: X-Original-To: apmail-felix-users-archive@minotaur.apache.org Delivered-To: apmail-felix-users-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 643391045C for ; Thu, 5 Dec 2013 15:07:08 +0000 (UTC) Received: (qmail 34995 invoked by uid 500); 5 Dec 2013 15:07:07 -0000 Delivered-To: apmail-felix-users-archive@felix.apache.org Received: (qmail 34731 invoked by uid 500); 5 Dec 2013 15:07:06 -0000 Mailing-List: contact users-help@felix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: users@felix.apache.org Delivered-To: mailing list users@felix.apache.org Received: (qmail 34719 invoked by uid 99); 5 Dec 2013 15:07:06 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 05 Dec 2013 15:07:06 +0000 X-ASF-Spam-Status: No, hits=2.2 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of bengt.rodehav@gmail.com designates 209.85.192.174 as permitted sender) Received: from [209.85.192.174] (HELO mail-pd0-f174.google.com) (209.85.192.174) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 05 Dec 2013 15:07:01 +0000 Received: by mail-pd0-f174.google.com with SMTP id y13so24918491pdi.33 for ; Thu, 05 Dec 2013 07:06:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:content-type; bh=CripUSlilNwl9wKZNEtDruiEkBgQlyIFnlf0yI0jKwQ=; b=cSPrNrMmVx5GAwjDuAALTESq142bd/EB6k+sagCoH4o1qWtsOSIsIJ7vEVJv4KM7e0 wD28K96gnmppA7lt0jV/9e60hazYosyMGIf68jY2NKEZUbKnJKyacP4FpnJzcCSTkyDv esEzKFQTJLf7LpcR22Z1KdNFdPnXx8xtAN0SX9EmOuFvpFeuWDR3uJuw8X4qygYPV1mi CeU6txgCcMMvMLkh2XR9DjSPFlLHUW9Eu8EUwAAFTmdTLzPZrWTF3aIFxheyVb/99bMf rLeNH1LAksJdG/qcMPd+6epHK3EzuAQCWo2dH+RKj9B1ip6Y2ajyDFR5HlxW59NGZOnu 3VJw== MIME-Version: 1.0 X-Received: by 10.68.190.33 with SMTP id gn1mr53654589pbc.48.1386256001076; Thu, 05 Dec 2013 07:06:41 -0800 (PST) Sender: bengt.rodehav@gmail.com Received: by 10.70.55.196 with HTTP; Thu, 5 Dec 2013 07:06:40 -0800 (PST) In-Reply-To: References: Date: Thu, 5 Dec 2013 16:06:40 +0100 X-Google-Sender-Auth: UYOMMOZTrGWpa8Crlzh4HKEUOyI Message-ID: Subject: Re: Fileinstall, problems with variable substitution and write back From: Bengt Rodehav To: "users@felix.apache.org" Content-Type: multipart/alternative; boundary=e89a8fb2088483167804eccae084 X-Virus-Checked: Checked by ClamAV on apache.org --e89a8fb2088483167804eccae084 Content-Type: text/plain; charset=ISO-8859-1 Just tested - works perfectly! Thanks a lot. When do you think the next release of FileInstall will take place? /Bengt 2013/12/5 Bengt Rodehav > OK - I'll test it (possibly tomorrow). > > /Bengt > > > 2013/12/5 Guillaume Nodet > >> Great. I've just committed a fix for FELIX-4339 >> >> >> 2013/12/5 Bengt Rodehav >> >> > Yes Guillaume - I agree with the behaviour in your test. All the >> properties >> > should have the same number of backslashes. >> > >> > /Bengt >> > >> > >> > 2013/12/5 Guillaume Nodet >> > >> > > Yeah, that's clearly FELIX-4339. >> > > I have a fix which makes the following test to succeed: >> > > >> > > public void testMultipleEscapes() >> > > { >> > > LinkedHashMap map1 = new LinkedHashMap> > > String>(); >> > > map1.put("a", "$\\\\{var}"); >> > > map1.put("abc", "${ab}c"); >> > > map1.put("ab", "${a}b"); >> > > InterpolationHelper.performSubstitution(map1); >> > > >> > > assertEquals("$\\{var}", map1.get("a")); >> > > assertEquals("$\\{var}b", map1.get("ab")); >> > > assertEquals("$\\{var}bc", map1.get("abc")); >> > > } >> > > >> > > Do we agree that's the behavior we should look for ? >> > > >> > > >> > > >> > > 2013/12/5 Bengt Rodehav >> > > >> > > > I've now tested FileInstall (and Utils) from trunk. Most things >> seem to >> > > > have been fixed now. In particular: >> > > > >> > > > - I haven't seen any "false" write back of configuration changes. >> > > > - The variables are not evaluated more than once which makes things >> > much >> > > > more deterministic >> > > > >> > > > I've found one remaining issue though. The unescaping is still not >> > > > deterministic. Every time the value of a variable is needed, it is >> > > > unescaped. I've tested this in Karaf 2.3.3 by putting a file called >> > > > test.cfg in the etc directory. I then issue the following command >> from >> > > the >> > > > Karaf console to see how my configuration is evaluated: >> > > > >> > > > config:list "(service.pid=test)" >> > > > >> > > > What I have found is that the following contents of test.cfg: >> > > > >> > > > a = $\\{var} >> > > > ab = ${a}b >> > > > abc = ${ab}c >> > > > >> > > > evaluates to: >> > > > >> > > > a = ${var} >> > > > ab = ${var}b >> > > > abc = ${var}bc >> > > > >> > > > This is correct (according to me). Note that testing it this way I >> need >> > > two >> > > > backslash characters instead of one as you used Guillaume. >> > > > >> > > > But, assume that I actually want my configuration to include a >> > backslash, >> > > > like this: >> > > > >> > > > a = $\\\\{var} >> > > > ab = ${a}b >> > > > abc = ${ab}c >> > > > >> > > > This evaluates to: >> > > > >> > > > a = $\{var} >> > > > ab = ${var}b >> > > > abc = ${var}bc >> > > > >> > > > Here the variable "a" is OK. But when ${a} is used in the other >> > > variables, >> > > > the backslash is unescaped and lost. This can be seen even more >> clearly >> > > if >> > > > you add two more backslashes. Like this: >> > > > >> > > > a = $\\\\\\\\{var} >> > > > ab = ${a}b >> > > > abc = ${ab}c >> > > > >> > > > We then get: >> > > > >> > > > a = $\\{var} >> > > > ab = $\{var}b >> > > > abc = ${var}bc >> > > > >> > > > Thus, every time the variable "a" is evaluated, a backslash is >> removed. >> > > > >> > > > It would be good if this could be fixed as well so that backslashes >> > could >> > > > be part of the configuration in a deterministic way. >> > > > >> > > > Note that the problematic configuration that I had no workaround for >> > now >> > > > works fine. If I put this in the configuration file: >> > > > >> > > > mydir=C:/temp >> > > > >> > > > >> > > >> > >> timestampedfile=$\\{file:onlyname\\}-$\\{date:now:yyyyMMddHHmmssSSS\\}.$\\{file:ext\\} >> > > > move=${mydir}/archive/$\\{date:now:yyyyMMdd\\}/${timestampedfile} >> > > > moveFailed=${mydir}/failed/${timestampedfile} >> > > > fromUri=file:${mydir}?move=${move}&moveFailed=${moveFailed} >> > > > >> > > > It is evaluated to: >> > > > >> > > > mydir = C:/temp >> > > > timestampedfile = >> > > > ${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext} >> > > > move = >> > > > >> > > > >> > > >> > >> C:/temp/archive/${date:now:yyyyMMdd}/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext} >> > > > moveFailed = >> > > > >> > >> C:/temp/failed/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext} >> > > > fromUri = >> > > > >> > > > >> > > >> > >> file:C:/temp?move=C:/temp/archive/${date:now:yyyyMMdd}/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext}&moveFailed=C:/temp/failed/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext} >> > > > >> > > > which is what I want. Note that I now only need two backslashes to >> make >> > > > this work. >> > > > >> > > > But, if I wanted some part of my configuration to actually contain a >> > > > backslash then I would run into trouble. >> > > > >> > > > /Bengt >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > 2013/12/4 Bengt Rodehav >> > > > >> > > > > I'll give it a shot. >> > > > > >> > > > > Thanks, >> > > > > >> > > > > /Bengt >> > > > > >> > > > > >> > > > > 2013/12/4 Guillaume Nodet >> > > > > >> > > > >> Not on the karaf side, but if you build felix utils, file >> install, >> > and >> > > > >> change the karaf version to refer to those new ones and rebuild, >> it >> > > > should >> > > > >> be ok. >> > > > >> >> > > > >> >> > > > >> 2013/12/4 Bengt Rodehav >> > > > >> >> > > > >> > Will definitely test this out. Thanks a lot. >> > > > >> > >> > > > >> > I assume everything is checked in? >> > > > >> > >> > > > >> > /Bengt >> > > > >> > Den 4 dec 2013 16:09 skrev "Guillaume Nodet" < >> gnodet@apache.org>: >> > > > >> > >> > > > >> > > Both are parts of the game. >> > > > >> > > The order actually was significant as shown by the test case >> > > mainly >> > > > >> > because >> > > > >> > > of the order difference between the java util Properties >> object >> > > and >> > > > >> the >> > > > >> > > felix Properties object. The first one is relies on a >> Hashtable >> > > > while >> > > > >> > the >> > > > >> > > second relies on a LinkedHashMap. >> > > > >> > > This is significant because of the way the substitution was >> > done. >> > > > >> > > if you start from a = $\{var}, ab = ${a}b, abc = ${ab}c >> > > > >> > > you had the following steps: >> > > > >> > > a = $\{var}, ab = ${a}b, abc = ${ab}c >> > > > >> > > a = $\{var}, ab = ${var}b, abc = ${ab}c >> > > > >> > > a = $\{var}, ab = ${var}b, abc = bc >> > > > >> > > The reason is that substitution were done using already >> > > substituted >> > > > >> > > variables, so when computing ${ab}c, it was using >> > > > >> > > ${ab}c >> > > > >> > > ${var}bc >> > > > >> > > bc >> > > > >> > > instead of >> > > > >> > > ${ab}c >> > > > >> > > ${a}bc >> > > > >> > > $\{var}bc >> > > > >> > > ${var}bc >> > > > >> > > So the problem wan't really the order of the values, but the >> > fact >> > > > that >> > > > >> > the >> > > > >> > > substitution was done using already substituted values, which >> > then >> > > > >> made >> > > > >> > the >> > > > >> > > order significant. >> > > > >> > > >> > > > >> > > Note that the result is now (and irrespective of the order of >> > the >> > > > >> lines): >> > > > >> > > a = ${var}, ab = ${var}b, abc = ${var}bc >> > > > >> > > >> > > > >> > > So I think the escaping is now more deterministic. Please >> give >> > it >> > > > >> some >> > > > >> > > testing and let me know if you still have problems in this >> area. >> > > > >> > > >> > > > >> > > 2013/12/4 Bengt Rodehav >> > > > >> > > >> > > > >> > > > BTW. I did some experimenting with declaring the >> properties in >> > > > >> > different >> > > > >> > > > order in the configuration file. It did not seem to >> matter. I >> > > was >> > > > >> under >> > > > >> > > the >> > > > >> > > > impression that the recursive variable substitution is what >> > > makes >> > > > it >> > > > >> > > > non-deterministic. >> > > > >> > > > >> > > > >> > > > If a property has been evaluated already it should not be >> > > > evaluated >> > > > >> > again >> > > > >> > > > because another layer of the escape characters will then be >> > > > removed. >> > > > >> > > > >> > > > >> > > > /Bengt >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > 2013/12/4 Bengt Rodehav >> > > > >> > > > >> > > > >> > > > > OK. >> > > > >> > > > > >> > > > >> > > > > >> > > > >> > > > > 2013/12/4 Guillaume Nodet >> > > > >> > > > > >> > > > >> > > > >> Unfortunately, it does not seem to be sufficient, I'm >> > > > >> investigating >> > > > >> > > > >> further >> > > > >> > > > >> >> > > > >> > > > >> >> > > > >> > > > >> 2013/12/4 Bengt Rodehav >> > > > >> > > > >> >> > > > >> > > > >> > I noticed that you seem to have fixed the issues I had >> > > > reported >> > > > >> > > > >> Guillaume. >> > > > >> > > > >> > Thanks a lot! Looking forward to the next release. >> > > > >> > > > >> > >> > > > >> > > > >> > /Bengt >> > > > >> > > > >> > >> > > > >> > > > >> > >> > > > >> > > > >> > 2013/12/2 Bengt Rodehav >> > > > >> > > > >> > >> > > > >> > > > >> > > Thanks Guillaume! >> > > > >> > > > >> > > >> > > > >> > > > >> > > >> > > > >> > > > >> > > 2013/12/2 Guillaume Nodet >> > > > >> > > > >> > > >> > > > >> > > > >> > >> I'll try to have a look at those today or tomorrow. >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> 2013/12/2 Bengt Rodehav >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> > I've replaced FELIX-4332 with FELIX-4338 and >> > > FELIX-4339. >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > I have attached a patch for FELIX-4338 and hope >> that >> > > > >> someone >> > > > >> > > can >> > > > >> > > > >> have >> > > > >> > > > >> > a >> > > > >> > > > >> > >> > look at it and possibly commit it. >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > FELIX-4339 is trickier but I would appreciate a >> > > > discussion >> > > > >> > > about >> > > > >> > > > >> how >> > > > >> > > > >> > >> this >> > > > >> > > > >> > >> > should be handled. >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > /Bengt >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > 2013/11/29 Bengt Rodehav >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> > > I've tested more with the proposed change in >> order >> > > to >> > > > >> stop >> > > > >> > > > >> > >> FileInstall to >> > > > >> > > > >> > >> > > incorrectly change the contents of the >> > configuration >> > > > >> file >> > > > >> > > > >> (problem >> > > > >> > > > >> > b) >> > > > >> > > > >> > >> > from >> > > > >> > > > >> > >> > > my previous post). It seems to work fine. I >> would >> > > > really >> > > > >> > like >> > > > >> > > > >> that >> > > > >> > > > >> > to >> > > > >> > > > >> > >> be >> > > > >> > > > >> > >> > > fixed. Would you like me to create a patch >> > atttached >> > > > to >> > > > >> the >> > > > >> > > > JIRA? >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > Problem a) is probably not trivial to fix. I've >> > > > >> > experimented >> > > > >> > > a >> > > > >> > > > >> lot >> > > > >> > > > >> > and >> > > > >> > > > >> > >> > > it's very hard for me to foresee how many >> escape >> > > > >> > characters I >> > > > >> > > > >> need >> > > > >> > > > >> > in >> > > > >> > > > >> > >> > > different circumstances. One real life example >> for >> > > me >> > > > is >> > > > >> > how >> > > > >> > > I >> > > > >> > > > >> > >> configure >> > > > >> > > > >> > >> > an >> > > > >> > > > >> > >> > > integration service that uses a Camel route >> > > > underneath. >> > > > >> If >> > > > >> > I >> > > > >> > > > put >> > > > >> > > > >> the >> > > > >> > > > >> > >> > > followiing contents in a test.cfg file: >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > *mydir=C:/temp* >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> > > > >> > > > >> >> > > > >> > > > >> > > > >> > > >> > > > >> > >> > > > >> >> > > > >> > > >> > >> *timestampedfile=$\\\\{file:onlyname\\\\}-$\\\\{date:now:yyyyMMddHHmmssSSS\\\\}.$\\\\{file:ext\\\\}* >> > > > >> > > > >> > >> > > >> > > > >> > > > >> >> > > > >> >> *move=${mydir}/archive/$\\{date:now:yyyyMMdd\\}/${timestampedfile}* >> > > > >> > > > >> > >> > > *moveFailed=${mydir}/failed/${timestampedfile}* >> > > > >> > > > >> > >> > > >> > > > >> > *fromUri=file:${mydir}?move=${move}&moveFailed=${moveFailed}* >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > And execute the following command: >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > *config:list "(service.pid=test)"* >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > I get the following output: >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> >> > > > >> >> *----------------------------------------------------------------* >> > > > >> > > > >> > >> > > *Pid: test* >> > > > >> > > > >> > >> > > *BundleLocation: null* >> > > > >> > > > >> > >> > > *Properties:* >> > > > >> > > > >> > >> > > * moveFailed = >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> > > > >> > > > >> >> > > > >> > > > >> > > > >> > > >> > > > >> > >> > > > >> >> > > > >> > > >> > >> C:/temp/failed/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext}* >> > > > >> > > > >> > >> > > * mydir = C:/temp* >> > > > >> > > > >> > >> > > * timestampedfile = >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > > $\{file:onlyname\}-$\{date:now:yyyyMMddHHmmssSSS\}.$\{file:ext\}* >> > > > >> > > > >> > >> > > * service.pid = test* >> > > > >> > > > >> > >> > > * fromUri = >> > > > >> > > > >> > >> > > >> > > > >> > > > >> >> > > > >> >> file:C:/temp?move=C:/temp/archive//-.&moveFailed=C:/temp/failed/-.* >> > > > >> > > > >> > >> > > * move = >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> > > > >> > > > >> >> > > > >> > > > >> > > > >> > > >> > > > >> > >> > > > >> >> > > > >> > > >> > >> C:/temp/archive/${date:now:yyyyMMdd}/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext}* >> > > > >> > > > >> > >> > > * felix.fileinstall.filename = >> > > > >> > > > >> > >> > > file:/C:/dev/karaf/connect/common/etc/test.cfg* >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > Thus, the variables "move" and "moveFailed" >> looks >> > > the >> > > > >> way I >> > > > >> > > > want >> > > > >> > > > >> but >> > > > >> > > > >> > >> the >> > > > >> > > > >> > >> > > final variable "fromUri" is messed up because >> of >> > an >> > > > >> extra >> > > > >> > > > >> variable >> > > > >> > > > >> > >> > > substitution. >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > I haven't managed to come up with any number of >> > > > >> backslashes >> > > > >> > > > that >> > > > >> > > > >> > will >> > > > >> > > > >> > >> > > produce the correct result for me. >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > The only workaround I have right now is to not >> use >> > > > >> > variables >> > > > >> > > at >> > > > >> > > > >> all. >> > > > >> > > > >> > >> It >> > > > >> > > > >> > >> > > does, however, make the configuration files >> > > extremely >> > > > >> > verbose >> > > > >> > > > and >> > > > >> > > > >> > it's >> > > > >> > > > >> > >> > easy >> > > > >> > > > >> > >> > > to introduce errors that way. >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > Presently, variable substitution is very >> > > unpredictable >> > > > >> > since >> > > > >> > > > it's >> > > > >> > > > >> > >> being >> > > > >> > > > >> > >> > > done in a recursive way. I would prefer doing >> it >> > in >> > > an >> > > > >> > > > iterative >> > > > >> > > > >> > >> manner >> > > > >> > > > >> > >> > to >> > > > >> > > > >> > >> > > make it predictable. E g "${a}" should always >> > > evaluate >> > > > >> to >> > > > >> > the >> > > > >> > > > >> same >> > > > >> > > > >> > >> value >> > > > >> > > > >> > >> > no >> > > > >> > > > >> > >> > > matter where in the configuration file it is >> > > > referenced. >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > /Bengt >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > > 2013/11/28 Bengt Rodehav >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > >> I've investigated this a bit more. There are >> > > actually >> > > > >> two >> > > > >> > > > >> different >> > > > >> > > > >> > >> > >> problems: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> a) The number of escape characters I need >> depends >> > > on >> > > > >> from >> > > > >> > > > where >> > > > >> > > > >> I >> > > > >> > > > >> > >> > >> reference the variable. For every indirection >> I >> > > need >> > > > to >> > > > >> > > double >> > > > >> > > > >> the >> > > > >> > > > >> > >> > number >> > > > >> > > > >> > >> > >> of backslashes. This also means that all uses >> of >> > a >> > > > >> > variable >> > > > >> > > > >> > >> containing >> > > > >> > > > >> > >> > >> escape characters must be used from the same >> > level >> > > of >> > > > >> > > > >> indirection. >> > > > >> > > > >> > A >> > > > >> > > > >> > >> bit >> > > > >> > > > >> > >> > >> complicated but it's due to the fact that all >> > > > variables >> > > > >> > are >> > > > >> > > > >> > evaluated >> > > > >> > > > >> > >> > >> dynamically. This means that unescaping can >> occur >> > > > >> several >> > > > >> > > > times. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> b) FileInstall incorrectly thinks that a >> > > > configuration >> > > > >> > > > property >> > > > >> > > > >> is >> > > > >> > > > >> > >> > >> changed and therefore overwrites the property >> > with >> > > > the >> > > > >> > > > evaluated >> > > > >> > > > >> > >> value. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> I think I've found the reason (and possibly a >> > > > >> solution) to >> > > > >> > > b). >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> In the ConfigInstaller.setConfig() method the >> > > > >> properties >> > > > >> > are >> > > > >> > > > >> read >> > > > >> > > > >> > >> from a >> > > > >> > > > >> > >> > >> configuration file and propagated as a >> > > configuration. >> > > > >> Here >> > > > >> > > is >> > > > >> > > > an >> > > > >> > > > >> > >> excerpt >> > > > >> > > > >> > >> > >> from that method: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> * final Properties p = new >> > > > >> Properties();* >> > > > >> > > > >> > >> > >> * in.mark(1);* >> > > > >> > > > >> > >> > >> * boolean isXml = in.read() == >> > '<';* >> > > > >> > > > >> > >> > >> * in.reset();* >> > > > >> > > > >> > >> > >> * if (isXml) {* >> > > > >> > > > >> > >> > >> * p.loadFromXML(in);* >> > > > >> > > > >> > >> > >> * } else {* >> > > > >> > > > >> > >> > >> * p.load(in);* >> > > > >> > > > >> > >> > >> * }* >> > > > >> > > > >> > >> > >> * >> > > > >> > > InterpolationHelper.performSubstitution((Map) >> > > > >> > > > >> p, >> > > > >> > > > >> > >> > >> context);* >> > > > >> > > > >> > >> > >> * ht.putAll(p);* >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> Note that the file is read using Java's >> standard >> > > > >> > Properties >> > > > >> > > > >> class. >> > > > >> > > > >> > >> The >> > > > >> > > > >> > >> > >> unescaping is also done by that class. Then, >> at >> > the >> > > > >> end, >> > > > >> > the >> > > > >> > > > >> > variable >> > > > >> > > > >> > >> > >> substitution is done as a separate call. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> Then look at the >> > > ConfigInstaller.configurationEvent() >> > > > >> > > method: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> * if (configurationEvent.getType() == >> > > > >> > > > >> > >> > >> ConfigurationEvent.CM_UPDATED)* >> > > > >> > > > >> > >> > >> * {* >> > > > >> > > > >> > >> > >> * try* >> > > > >> > > > >> > >> > >> * {* >> > > > >> > > > >> > >> > >> * Configuration config = >> > > > >> > > > >> > >> > >> getConfigurationAdmin().getConfiguration(* >> > > > >> > > > >> > >> > >> * >> > > > >> > > > >> > >> > configurationEvent.getPid(),* >> > > > >> > > > >> > >> > >> * >> > > > >> > > > >> > >> > >> configurationEvent.getFactoryPid());* >> > > > >> > > > >> > >> > >> * Dictionary dict = >> > > > >> > config.getProperties();* >> > > > >> > > > >> > >> > >> * String fileName = (String) >> > > dict.get( >> > > > >> > > > >> > >> > >> DirectoryWatcher.FILENAME );* >> > > > >> > > > >> > >> > >> * File file = fileName != null >> ? >> > > > >> > > > >> > >> fromConfigKey(fileName) >> > > > >> > > > >> > >> > : >> > > > >> > > > >> > >> > >> null;* >> > > > >> > > > >> > >> > >> * if( file != null && >> > file.isFile() >> > > > ) >> > > > >> {* >> > > > >> > > > >> > >> > >> * if( fileName.endsWith( >> > ".cfg" >> > > ) >> > > > )* >> > > > >> > > > >> > >> > >> * {* >> > > > >> > > > >> > >> > >> * >> > > > >> > > > >> > org.apache.felix.utils.properties.Properties >> > > > >> > > > >> > >> > >> props = new >> > > > >> org.apache.felix.utils.properties.Properties( >> > > > >> > > > file, >> > > > >> > > > >> > >> context >> > > > >> > > > >> > >> > );* >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> Note that now the configuration file is read >> > using >> > > > >> > > > >> > >> > >> org.apache.felix.utils.properties.Properties >> > class. >> > > > It >> > > > >> > turns >> > > > >> > > > out >> > > > >> > > > >> > that >> > > > >> > > > >> > >> > they >> > > > >> > > > >> > >> > >> don't produce identical results. I haven't >> > > > investigated >> > > > >> > > > exactly >> > > > >> > > > >> how >> > > > >> > > > >> > >> they >> > > > >> > > > >> > >> > >> differ but they do. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> A simple test: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> 1. Create a configuration file with the >> following >> > > > >> content: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> a=$\\\\{var} >> > > > >> > > > >> > >> > >> ab=${a}b >> > > > >> > > > >> > >> > >> abc=${ab}c >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> 2. Add the following line at the end: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> d=foo >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> 3. FileInstall will now incorrectly change the >> > > > >> contents of >> > > > >> > > the >> > > > >> > > > >> > >> > >> configuration file to: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> a=$\\\\{var} >> > > > >> > > > >> > >> > >> ab=${a}b >> > > > >> > > > >> > >> > >> abc = ${var}bc >> > > > >> > > > >> > >> > >> d=foo >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> Now if I change the >> ConfigInstaller.setConfig() >> > > > method >> > > > >> to >> > > > >> > > the >> > > > >> > > > >> > >> following: >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> *org.apache.felix.utils.properties.Properties >> p = >> > > new >> > > > >> > > > >> > >> > >> org.apache.felix.utils.properties.Properties( >> f, >> > > > >> context >> > > > >> > );* >> > > > >> > > > >> > >> > >> >> *InterpolationHelper.performSubstitution((Map) p, >> > > > >> > context);* >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> Then FileInstall will not incorrectly change >> the >> > > > >> contents >> > > > >> > of >> > > > >> > > > the >> > > > >> > > > >> > >> > >> configuration file. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> I propose to do this change in order to solve >> > > problem >> > > > >> b) >> > > > >> > > > above. >> > > > >> > > > >> I >> > > > >> > > > >> > >> > >> appreciate if you have any thoughts on this. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> I realize that problem a) is trickier due to >> the >> > > > >> dynamic >> > > > >> > > > nature >> > > > >> > > > >> of >> > > > >> > > > >> > >> > >> variable substitution. I haven't yet >> determined >> > > how I >> > > > >> > think >> > > > >> > > > the >> > > > >> > > > >> > >> escape >> > > > >> > > > >> > >> > >> characters should be handled but the current >> > > > situation >> > > > >> is >> > > > >> > > not >> > > > >> > > > >> > ideal. >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> /Bengt >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >> 2013/11/28 Bengt Rodehav >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > >>> JIRA created: >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >>> >> > https://issues.apache.org/jira/browse/FELIX-4332 >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >>> /Bengt >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >>> 2013/11/28 Bengt Rodehav >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >>>> I've come up with easily reproducable errors >> > > using >> > > > >> Karaf >> > > > >> > > > >> 2.3.3: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> - Install a fresh Karaf 2.3.3 >> > > > >> > > > >> > >> > >>>> - Add the following line to >> > > etc/custom.properties: >> > > > >> > > > >> > >> > >>>> felix.fileinstall.enableConfigSave = true >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> Create a file etc/test.cfg with the >> following >> > > > >> contents: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> a=$\\{var} >> > > > >> > > > >> > >> > >>>> ab=${a}b >> > > > >> > > > >> > >> > >>>> abc=${ab}c >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> I expect this to be evaluated to: >> > > > >> > > > >> > >> > >>>> a=$\{var} >> > > > >> > > > >> > >> > >>>> ab=$\{var}b >> > > > >> > > > >> > >> > >>>> abc=$\{var}bc >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> But if I execute the Karaf command: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> config:list "(service.pid=test)" >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> I get: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> >> > > > ---------------------------------------------------------------- >> > > > >> > > > >> > >> > >>>> Pid: test >> > > > >> > > > >> > >> > >>>> BundleLocation: null >> > > > >> > > > >> > >> > >>>> Properties: >> > > > >> > > > >> > >> > >>>> service.pid = test >> > > > >> > > > >> > >> > >>>> a = ${var} >> > > > >> > > > >> > >> > >>>> abc = bc >> > > > >> > > > >> > >> > >>>> felix.fileinstall.filename = >> > > > >> > > > >> > >> > >>>> >> > > file:/C:/dev/Karaf/apache-karaf-2.3.3/etc/test.cfg >> > > > >> > > > >> > >> > >>>> ab = b >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> My interpretation of this is that the >> variable >> > > "a" >> > > > >> has >> > > > >> > > been >> > > > >> > > > >> > >> correctly >> > > > >> > > > >> > >> > >>>> evaluated. But, when evalutating the >> variable >> > > "ab" >> > > > it >> > > > >> > > seems >> > > > >> > > > >> that >> > > > >> > > > >> > >> the >> > > > >> > > > >> > >> > >>>> variable "a" is evaluated again despite the >> > fact >> > > > >> that it >> > > > >> > > has >> > > > >> > > > >> > >> already >> > > > >> > > > >> > >> > been >> > > > >> > > > >> > >> > >>>> evaluated. FileInstall now looks for the >> value >> > > of a >> > > > >> > > variable >> > > > >> > > > >> > called >> > > > >> > > > >> > >> > "var" >> > > > >> > > > >> > >> > >>>> which evalutes to an empty string because >> there >> > > is >> > > > no >> > > > >> > such >> > > > >> > > > >> > >> variable. >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> The variable "abc" consequently evaluates to >> > "bc" >> > > > >> since >> > > > >> > > the >> > > > >> > > > >> > >> variable >> > > > >> > > > >> > >> > >>>> "ab" has been evaluated to "b". >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> To make it even worse, now change the first >> row >> > > in >> > > > >> > > test.cfg >> > > > >> > > > >> to: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> a=$\\\\{var} >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> We now get: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> >> > > > ---------------------------------------------------------------- >> > > > >> > > > >> > >> > >>>> Pid: test >> > > > >> > > > >> > >> > >>>> BundleLocation: null >> > > > >> > > > >> > >> > >>>> Properties: >> > > > >> > > > >> > >> > >>>> service.pid = test >> > > > >> > > > >> > >> > >>>> a = $\{var} >> > > > >> > > > >> > >> > >>>> abc = ${var}bc >> > > > >> > > > >> > >> > >>>> felix.fileinstall.filename = >> > > > >> > > > >> > >> > >>>> >> > > file:/C:/dev/Karaf/apache-karaf-2.3.3/etc/test.cfg >> > > > >> > > > >> > >> > >>>> ab = ${var}b >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> Thus we get the same phenomenom. The >> variable >> > "a" >> > > > is >> > > > >> > > > evaluated >> > > > >> > > > >> > >> > >>>> differently if it is evaluated on its own >> or as >> > > > part >> > > > >> of >> > > > >> > > > >> another >> > > > >> > > > >> > >> > expression. >> > > > >> > > > >> > >> > >>>> But, due to having configured FileInstall to >> > > write >> > > > >> back >> > > > >> > > > >> changes, >> > > > >> > > > >> > >> the >> > > > >> > > > >> > >> > >>>> contents of the test.cfg is now changed by >> > > > >> FileInstall >> > > > >> > > > despite >> > > > >> > > > >> > the >> > > > >> > > > >> > >> > fact >> > > > >> > > > >> > >> > >>>> that the configuration has not changed at >> all. >> > > The >> > > > >> > > contents >> > > > >> > > > of >> > > > >> > > > >> > >> > test.cfg is >> > > > >> > > > >> > >> > >>>> now: >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> a=$\\\\{var} >> > > > >> > > > >> > >> > >>>> ab=${a}b >> > > > >> > > > >> > >> > >>>> abc = ${var}bc >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> The "abc" variable has been altered. >> > FileInstall >> > > > has >> > > > >> > > > >> incorrectly >> > > > >> > > > >> > >> > >>>> determined that its value has changed. >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> This is clearly a bug. I will create a JIRA. >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> /Bengt >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>> 2013/11/26 Bengt Rodehav > > >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>>>> I'm using Apache Karaf 2.3.3 which comes >> with >> > > > >> > FileInstall >> > > > >> > > > >> > 3.2.6. I >> > > > >> > > > >> > >> > >>>>> have set the >> > felix.fileinstall.enableConfigSave >> > > > >> > property >> > > > >> > > to >> > > > >> > > > >> true >> > > > >> > > > >> > >> in >> > > > >> > > > >> > >> > order >> > > > >> > > > >> > >> > >>>>> to have FileInstall write back >> configuration >> > > > >> changes to >> > > > >> > > the >> > > > >> > > > >> > file. >> > > > >> > > > >> > >> > Normally >> > > > >> > > > >> > >> > >>>>> all configuration changes are done by >> editing >> > > the >> > > > >> > > > >> configuration >> > > > >> > > > >> > >> file >> > > > >> > > > >> > >> > but >> > > > >> > > > >> > >> > >>>>> there is one property that I change >> > > > programmatically >> > > > >> > > using >> > > > >> > > > >> > >> > ConfigAdmin (an >> > > > >> > > > >> > >> > >>>>> "enable" property to start/stop my >> service). I >> > > am >> > > > >> > > dependent >> > > > >> > > > >> on >> > > > >> > > > >> > >> that >> > > > >> > > > >> > >> > >>>>> property being persisted in the >> configuration >> > > file >> > > > >> > which >> > > > >> > > is >> > > > >> > > > >> why >> > > > >> > > > >> > I >> > > > >> > > > >> > >> > set the >> > > > >> > > > >> > >> > >>>>> enableConfigSave property to true. >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> When configuring FileInstall to write back >> > > > >> > configuration >> > > > >> > > > >> changes >> > > > >> > > > >> > >> to >> > > > >> > > > >> > >> > >>>>> the configuration file, it is important >> that >> > > > >> variables >> > > > >> > > are >> > > > >> > > > >> not >> > > > >> > > > >> > >> > substituted >> > > > >> > > > >> > >> > >>>>> for the evaluated value. This normally >> works >> > > since >> > > > >> > > > >> FileInstall >> > > > >> > > > >> > >> > evalutates >> > > > >> > > > >> > >> > >>>>> the property in the configuration file and >> > > > compares >> > > > >> it >> > > > >> > > with >> > > > >> > > > >> the >> > > > >> > > > >> > >> > >>>>> configuration admin's value. If they are >> the >> > > same, >> > > > >> the >> > > > >> > > > value >> > > > >> > > > >> in >> > > > >> > > > >> > >> the >> > > > >> > > > >> > >> > >>>>> configuration file is kept unchanged. >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> However, when using the escape character >> this >> > is >> > > > >> > broken. >> > > > >> > > In >> > > > >> > > > >> my >> > > > >> > > > >> > >> case >> > > > >> > > > >> > >> > >>>>> I'm using Apache Camel underneath. When >> > > > configuring >> > > > >> > > routes >> > > > >> > > > >> via >> > > > >> > > > >> > the >> > > > >> > > > >> > >> > config >> > > > >> > > > >> > >> > >>>>> admin, I sometimes need to set a value to >> > > > >> > > > >> > >> > >>>>> "${expression-to-be-evaluated-by-camel}". I >> > > > >> therefore >> > > > >> > > > escape >> > > > >> > > > >> the >> > > > >> > > > >> > >> "{" >> > > > >> > > > >> > >> > and >> > > > >> > > > >> > >> > >>>>> "}" to stop FileInstall from trying to >> > evaluate >> > > > the >> > > > >> > > > >> expression. >> > > > >> > > > >> > >> Like >> > > > >> > > > >> > >> > this: >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> $\\{expression-to-be-evaluated-by-camel\\} >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> This also normally works but not when I >> have >> > an >> > > > >> > > > indirection. >> > > > >> > > > >> E g >> > > > >> > > > >> > >> when >> > > > >> > > > >> > >> > >>>>> specifying the following: >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> a=$\\{var} >> > > > >> > > > >> > >> > >>>>> ab=${a}b >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> FileInstall will change the configuration >> file >> > > to: >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> a=$\\{var} >> > > > >> > > > >> > >> > >>>>> ab = ${var}b >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> Note that the variable "ab" has now been >> > > expanded >> > > > >> and >> > > > >> > > > written >> > > > >> > > > >> > >> back to >> > > > >> > > > >> > >> > >>>>> the configuration file even if neither of >> the >> > > > >> variables >> > > > >> > > "a" >> > > > >> > > > >> and >> > > > >> > > > >> > >> "ab" >> > > > >> > > > >> > >> > have >> > > > >> > > > >> > >> > >>>>> been changed. >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> I think this is because FileInstall does >> the >> > > > >> following: >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> 1. Calculates the value of "a" to "$\{var} >> > > > >> > > > >> > >> > >>>>> 2. Calculates the value of "b" to "${var}b >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> Note that every evaluation will perform >> > > > >> "unescaping". >> > > > >> > > This >> > > > >> > > > >> means >> > > > >> > > > >> > >> that >> > > > >> > > > >> > >> > >>>>> an extra "unescaping" will be done for >> every >> > > > >> > indirection >> > > > >> > > > >> which >> > > > >> > > > >> > >> fools >> > > > >> > > > >> > >> > >>>>> FileInstall into thinking that the property >> > has >> > > > been >> > > > >> > > > changed. >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> I'm not exactly sure how this should be >> fixed >> > in >> > > > >> > > > FileInstall. >> > > > >> > > > >> > One >> > > > >> > > > >> > >> > idea >> > > > >> > > > >> > >> > >>>>> is to never "unescape" already evaluated >> > > > variables. >> > > > >> > > > Actually >> > > > >> > > > >> I >> > > > >> > > > >> > >> think >> > > > >> > > > >> > >> > this >> > > > >> > > > >> > >> > >>>>> is probably what would fix this... >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> Does anybody have any ideas about this? >> > Should I >> > > > >> > create a >> > > > >> > > > >> JIRA? >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> /Bengt >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>>> >> > > > >> > > > >> > >> > >>>> >> > > > >> > > > >> > >> > >>> >> > > > >> > > > >> > >> > >> >> > > > >> > > > >> > >> > > >> > > > >> > > > >> > >> > >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> -- >> > > > >> > > > >> > >> ----------------------- >> > > > >> > > > >> > >> Guillaume Nodet >> > > > >> > > > >> > >> ------------------------ >> > > > >> > > > >> > >> Red Hat, Open Source Integration >> > > > >> > > > >> > >> >> > > > >> > > > >> > >> Email: gnodet@redhat.com >> > > > >> > > > >> > >> Web: http://fusesource.com >> > > > >> > > > >> > >> Blog: http://gnodet.blogspot.com/ >> > > > >> > > > >> > >> >> > > > >> > > > >> > > >> > > > >> > > > >> > > >> > > > >> > > > >> > >> > > > >> > > > >> >> > > > >> > > > > >> > > > >> > > > > >> > > > >> > > > >> > > > >> > > >> > > > >> > >> > > > >> >> > > > > >> > > > > >> > > > >> > > >> > >> > > --e89a8fb2088483167804eccae084--