| Message ID | 1488550733-3956-6-git-send-email-wg@grandegger.com |
|---|---|
| State | Superseded |
| Headers | show |
On 03-03-17 15:18, Wolfgang Grandegger wrote: > It will create the script "relocate-toolchain.sh" in $HOST_DIR/usr/ > allowing to adjust the path to the toolchain directory in all text > files after it has been moved to a new location. > > Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> > --- > support/scripts/create-relocation-script | 72 ++++++++++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100755 support/scripts/create-relocation-script > > diff --git a/support/scripts/create-relocation-script b/support/scripts/create-relocation-script > new file mode 100755 > index 0000000..0ceaeb2 > --- /dev/null > +++ b/support/scripts/create-relocation-script > @@ -0,0 +1,72 @@ > +#!/bin/sh > + > +# Creates the script "relocate-toolchain.sh" and the "location" file in > +# the buildroot toolchain (host/usr). After copying that toolchain tree > +# to a new location, the script in the top directory should be executed > +# to relocate the toolchain. It actually will replace the string of the > +# old location with the new one in *all* text files. > + > +if [ "$#" -ne 1 -o ! -d "$1" ]; then > + echo "Usage: $0 <buildroot-host-directory-path>" > + exit 1 > +fi > + > +# The toolchain is in buildroot's "host/usr" > +TOOLCHAINDIR="$1/usr" > +# We create the script in the top directory of the toolchain > +SCRIPTFILE="${TOOLCHAINDIR}/relocate-toolchain.sh" I think installing the script in host/usr/bin would be more appropriate. > +# File holding the location of the buildroot toolchain > +LOCATIONFILE="share/buildroot/toolchain-location" > + > +echo "Creating ${SCRIPTFILE} for toolchain relocation ..." > + > +cat << EOF > "${SCRIPTFILE}" Instead of using cat to instantiate the template, it's easier to understand what happens by creating a template file (support/misc/relocate-toolchain.sh.in) and using sed to instantiate the variables that need to be instantiated. Such a sed command can typically be called directly from Makefile, without helper script. However, as far as I can see, the only thing that is instantiated in this script is LOCATIONFILE. But that variable is actually constant, so it could be coded directly in the script. So why not just $(INSTALL) the script itself, like is done for e.g. support/misc/target-dir-warning.txt ? > +#!/bin/sh > +# > +# Automatically generated by $0: don't edit > +# > +if [ "\$#" -ne 0 ]; then > + echo "Run this script to relocate the buildroot toolchain at that location" > + exit 1 > +fi > + > +FILEPATH="\$(readlink -f \$0)" > +NEWPATH="\$(dirname \${FILEPATH})" > + > +cd \${NEWPATH} > +if [ ! -r ./${LOCATIONFILE} ]; then > + echo "Previous location of the buildroot toolchain not found!" > + exit 1 > +fi > +OLDPATH="\$(cat ./${LOCATIONFILE})" > + > +if [ \${NEWPATH} = \${OLDPATH} ]; then > + echo "This buildroot toolchain has already been relocated!" > + exit 0 > +fi > + > +echo "Relocating the buildroot toolchain from \${OLDPATH} to \${NEWPATH} ..." > + > +# Make sure file uses the right language > +export LC_ALL=C > +# Replace the old path with the new one in all text files > +for FILE in \$(grep -r "\${OLDPATH}" . -l ) ; do We typically collect options before the regex, so grep -r -l ... I think that there are a few packages that might install something with spaces (though probably not in $HOST_DIR), so find -exec is probably more appropriate. Unfortunately, that doesn't allow you to do a test with a pipe however so a different solution would be needed to support files with spaces. > + if [ ! -h \${FILE} ] && [ -n "\$(file -b --mime-type \${FILE} | grep '^text/' )" ] && [ "\${FILE}" != "./${LOCATIONFILE}" ] > + then > + sed -i s,"\${OLDPATH}",\${NEWPATH},g \${FILE} > + fi > +done > + > +# Finally, we update the location file itself > +sed -i s,"\${OLDPATH}",\${NEWPATH},g ./${LOCATIONFILE} Why do this separately? It works within the loop above as well, doesn't it? > + > +# Check if the path substitution did work properly > +if [ "\${NEWPATH}" != "\$(cat ./${LOCATIONFILE})" ]; then > + echo "Something went wrong with substituting the path!" > + echo "Please choose another location for your toolchain!" > +fi Is there any reason why this error handling is needed? The only thing I can imagine is concurrently running two instances of the script, but otherwise I see no way that the sed could fail... Regards, Arnout > +EOF > +chmod +x "${SCRIPTFILE}" > +mkdir -p $(dirname "$1/usr/${LOCATIONFILE}") > +# Finally write the toolchain location > +echo "$TOOLCHAINDIR" > "${TOOLCHAINDIR}/${LOCATIONFILE}" >
Hello, Am 16.03.2017 um 18:51 schrieb Arnout Vandecappelle: > > > On 03-03-17 15:18, Wolfgang Grandegger wrote: >> It will create the script "relocate-toolchain.sh" in $HOST_DIR/usr/ >> allowing to adjust the path to the toolchain directory in all text >> files after it has been moved to a new location. >> >> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> >> --- >> support/scripts/create-relocation-script | 72 ++++++++++++++++++++++++++++++++ >> 1 file changed, 72 insertions(+) >> create mode 100755 support/scripts/create-relocation-script >> >> diff --git a/support/scripts/create-relocation-script b/support/scripts/create-relocation-script >> new file mode 100755 >> index 0000000..0ceaeb2 >> --- /dev/null >> +++ b/support/scripts/create-relocation-script >> @@ -0,0 +1,72 @@ >> +#!/bin/sh >> + >> +# Creates the script "relocate-toolchain.sh" and the "location" file in >> +# the buildroot toolchain (host/usr). After copying that toolchain tree >> +# to a new location, the script in the top directory should be executed >> +# to relocate the toolchain. It actually will replace the string of the >> +# old location with the new one in *all* text files. >> + >> +if [ "$#" -ne 1 -o ! -d "$1" ]; then >> + echo "Usage: $0 <buildroot-host-directory-path>" >> + exit 1 >> +fi >> + >> +# The toolchain is in buildroot's "host/usr" >> +TOOLCHAINDIR="$1/usr" >> +# We create the script in the top directory of the toolchain >> +SCRIPTFILE="${TOOLCHAINDIR}/relocate-toolchain.sh" > > I think installing the script in host/usr/bin would be more appropriate. I thought it's more visible/present in the root directory. >> +# File holding the location of the buildroot toolchain >> +LOCATIONFILE="share/buildroot/toolchain-location" >> + >> +echo "Creating ${SCRIPTFILE} for toolchain relocation ..." >> + >> +cat << EOF > "${SCRIPTFILE}" > > Instead of using cat to instantiate the template, it's easier to understand > what happens by creating a template file > (support/misc/relocate-toolchain.sh.in) and using sed to instantiate the > variables that need to be instantiated. Such a sed command can typically be > called directly from Makefile, without helper script. I just see that you are looking to v1 of my RFC patch series. I have sent v2 yesterday. In v2 I use the name "SDK" instead of "toolchain". IIUC, the SDK is under "HOST_DIR" and the toolchain under "HOST_DIR/usr". And we want to relocate the SDK, right? > > However, as far as I can see, the only thing that is instantiated in this > script is LOCATIONFILE. But that variable is actually constant, so it could be > coded directly in the script. So why not just $(INSTALL) the script itself, like > is done for e.g. support/misc/target-dir-warning.txt ? Will adapt that solution. I was just not sure about the locations. >> +#!/bin/sh >> +# >> +# Automatically generated by $0: don't edit >> +# >> +if [ "\$#" -ne 0 ]; then >> + echo "Run this script to relocate the buildroot toolchain at that location" >> + exit 1 >> +fi >> + >> +FILEPATH="\$(readlink -f \$0)" >> +NEWPATH="\$(dirname \${FILEPATH})" >> + >> +cd \${NEWPATH} >> +if [ ! -r ./${LOCATIONFILE} ]; then >> + echo "Previous location of the buildroot toolchain not found!" >> + exit 1 >> +fi >> +OLDPATH="\$(cat ./${LOCATIONFILE})" >> + >> +if [ \${NEWPATH} = \${OLDPATH} ]; then >> + echo "This buildroot toolchain has already been relocated!" >> + exit 0 >> +fi >> + >> +echo "Relocating the buildroot toolchain from \${OLDPATH} to \${NEWPATH} ..." >> + >> +# Make sure file uses the right language >> +export LC_ALL=C >> +# Replace the old path with the new one in all text files >> +for FILE in \$(grep -r "\${OLDPATH}" . -l ) ; do > > We typically collect options before the regex, so grep -r -l ... > > I think that there are a few packages that might install something with spaces > (though probably not in $HOST_DIR), so find -exec is probably more appropriate. > Unfortunately, that doesn't allow you to do a test with a pipe however so a > different solution would be needed to support files with spaces. Yes, these magic spaces in file names. I will try using "find ... -exec". > >> + if [ ! -h \${FILE} ] && [ -n "\$(file -b --mime-type \${FILE} | grep '^text/' )" ] && [ "\${FILE}" != "./${LOCATIONFILE}" ] >> + then >> + sed -i s,"\${OLDPATH}",\${NEWPATH},g \${FILE} >> + fi >> +done >> + >> +# Finally, we update the location file itself >> +sed -i s,"\${OLDPATH}",\${NEWPATH},g ./${LOCATIONFILE} > > Why do this separately? It works within the loop above as well, doesn't it? If the script is interrupted with ^C, you can through away the tree. If I update the location file at the end, just re-running the script will work. >> + >> +# Check if the path substitution did work properly >> +if [ "\${NEWPATH}" != "\$(cat ./${LOCATIONFILE})" ]; then >> + echo "Something went wrong with substituting the path!" >> + echo "Please choose another location for your toolchain!" >> +fi > > Is there any reason why this error handling is needed? The only thing I can > imagine is concurrently running two instances of the script, but otherwise I see > no way that the sed could fail... If the path /a/b/c/a/b/c is copied to /a/b/c, relocation (substitution) will not be correct. Maybe too much paranoia. Wolfgang.
On 17-03-17 08:10, Wolfgang Grandegger wrote: > Am 16.03.2017 um 18:51 schrieb Arnout Vandecappelle: >> On 03-03-17 15:18, Wolfgang Grandegger wrote: [snip] >>> +# The toolchain is in buildroot's "host/usr" >>> +TOOLCHAINDIR="$1/usr" >>> +# We create the script in the top directory of the toolchain >>> +SCRIPTFILE="${TOOLCHAINDIR}/relocate-toolchain.sh" >> >> I think installing the script in host/usr/bin would be more appropriate. > > I thought it's more visible/present in the root directory. Well, the root directory would be host, not host/usr, right? >>> +# File holding the location of the buildroot toolchain >>> +LOCATIONFILE="share/buildroot/toolchain-location" >>> + >>> +echo "Creating ${SCRIPTFILE} for toolchain relocation ..." >>> + >>> +cat << EOF > "${SCRIPTFILE}" >> >> Instead of using cat to instantiate the template, it's easier to understand >> what happens by creating a template file >> (support/misc/relocate-toolchain.sh.in) and using sed to instantiate the >> variables that need to be instantiated. Such a sed command can typically be >> called directly from Makefile, without helper script. > > I just see that you are looking to v1 of my RFC patch series. I have sent v2 > yesterday. Yes, I wrote this mail while offline and didn't see your new series until later. > In v2 I use the name "SDK" instead of "toolchain". > IIUC, the SDK is under "HOST_DIR" and the toolchain under "HOST_DIR/usr". And we > want to relocate the SDK, right? There is no clear distinction between SDK or toolchain - SDK is a little bit more, it can contain other host tools e.g. genimage. The usr part is just historical accident that we are going to remove (when someone (i.e. me :-) finally finds the time to do it). Anyway, for this series SDK is indeed a better term than toolchain. >> However, as far as I can see, the only thing that is instantiated in this >> script is LOCATIONFILE. But that variable is actually constant, so it could be >> coded directly in the script. So why not just $(INSTALL) the script itself, like >> is done for e.g. support/misc/target-dir-warning.txt ? > > Will adapt that solution. I was just not sure about the locations. > [snip] >>> +# Finally, we update the location file itself >>> +sed -i s,"\${OLDPATH}",\${NEWPATH},g ./${LOCATIONFILE} >> >> Why do this separately? It works within the loop above as well, doesn't it? > > If the script is interrupted with ^C, you can through away the tree. If I update > the location file at the end, just re-running the script will work. That's a very good reason. Add a comment above it to explain. > >>> + >>> +# Check if the path substitution did work properly >>> +if [ "\${NEWPATH}" != "\$(cat ./${LOCATIONFILE})" ]; then >>> + echo "Something went wrong with substituting the path!" >>> + echo "Please choose another location for your toolchain!" >>> +fi >> >> Is there any reason why this error handling is needed? The only thing I can >> imagine is concurrently running two instances of the script, but otherwise I see >> no way that the sed could fail... > > If the path /a/b/c/a/b/c is copied to /a/b/c, relocation (substitution) will not > be correct. Maybe too much paranoia. No, a very good point actually! Maybe then a better approach is to first generate the new location file (in a temporary new file), check that it looks OK, and don't continue if it is not OK. Do add a comment to to explain the risk. You found this one instance where it goes wrong, but there may be others as well. Regards, Arnout
Am 17.03.2017 um 16:50 schrieb Arnout Vandecappelle: > > > On 17-03-17 08:10, Wolfgang Grandegger wrote: >> Am 16.03.2017 um 18:51 schrieb Arnout Vandecappelle: >>> On 03-03-17 15:18, Wolfgang Grandegger wrote: > [snip] >>>> +# The toolchain is in buildroot's "host/usr" >>>> +TOOLCHAINDIR="$1/usr" >>>> +# We create the script in the top directory of the toolchain >>>> +SCRIPTFILE="${TOOLCHAINDIR}/relocate-toolchain.sh" >>> >>> I think installing the script in host/usr/bin would be more appropriate. >> >> I thought it's more visible/present in the root directory. > > Well, the root directory would be host, not host/usr, right? Of course! >>>> +# File holding the location of the buildroot toolchain >>>> +LOCATIONFILE="share/buildroot/toolchain-location" >>>> + >>>> +echo "Creating ${SCRIPTFILE} for toolchain relocation ..." >>>> + >>>> +cat << EOF > "${SCRIPTFILE}" >>> >>> Instead of using cat to instantiate the template, it's easier to understand >>> what happens by creating a template file >>> (support/misc/relocate-toolchain.sh.in) and using sed to instantiate the >>> variables that need to be instantiated. Such a sed command can typically be >>> called directly from Makefile, without helper script. >> >> I just see that you are looking to v1 of my RFC patch series. I have sent v2 >> yesterday. > > Yes, I wrote this mail while offline and didn't see your new series until later. No problem. At the very first moment, I thought I sent the old series. >> In v2 I use the name "SDK" instead of "toolchain". >> IIUC, the SDK is under "HOST_DIR" and the toolchain under "HOST_DIR/usr". And we >> want to relocate the SDK, right? > > There is no clear distinction between SDK or toolchain - SDK is a little bit > more, it can contain other host tools e.g. genimage. The usr part is just > historical accident that we are going to remove (when someone (i.e. me :-) > finally finds the time to do it). > > Anyway, for this series SDK is indeed a better term than toolchain. > >>> However, as far as I can see, the only thing that is instantiated in this >>> script is LOCATIONFILE. But that variable is actually constant, so it could be >>> coded directly in the script. So why not just $(INSTALL) the script itself, like >>> is done for e.g. support/misc/target-dir-warning.txt ? >> >> Will adapt that solution. I was just not sure about the locations. >> > [snip] # Replace the old path with the new one in all text files while read FILE ; do if file -b --mime-type "${FILE}" | grep -q '^text/' && [ "${FILE}" != "./usr/share/buildroot/sdk-location" ] then sed -i s,"${OLDPATH}","${NEWPATH}",g "${FILE}" fi; done < <(grep -lr "${OLDPATH}" .) As grep -rl is very fast; I now use the following construct handling file with names with space corectly. BTW, is ',' is also a valid name character. Are all characters allowed for file names? Is there a known trick? >>>> +# Finally, we update the location file itself >>>> +sed -i s,"\${OLDPATH}",\${NEWPATH},g ./${LOCATIONFILE} >>> >>> Why do this separately? It works within the loop above as well, doesn't it? >> >> If the script is interrupted with ^C, you can through away the tree. If I update >> the location file at the end, just re-running the script will work. > > That's a very good reason. Add a comment above it to explain. OK. >> >>>> + >>>> +# Check if the path substitution did work properly >>>> +if [ "\${NEWPATH}" != "\$(cat ./${LOCATIONFILE})" ]; then >>>> + echo "Something went wrong with substituting the path!" >>>> + echo "Please choose another location for your toolchain!" >>>> +fi >>> >>> Is there any reason why this error handling is needed? The only thing I can >>> imagine is concurrently running two instances of the script, but otherwise I see >>> no way that the sed could fail... >> >> If the path /a/b/c/a/b/c is copied to /a/b/c, relocation (substitution) will not >> be correct. Maybe too much paranoia. > > No, a very good point actually! Maybe then a better approach is to first > generate the new location file (in a temporary new file), check that it looks > OK, and don't continue if it is not OK. +1 > Do add a comment to to explain the risk. You found this one instance where it > goes wrong, but there may be others as well. OK. Wolfgang.
On 17-03-17 18:15, Wolfgang Grandegger wrote: > Am 17.03.2017 um 16:50 schrieb Arnout Vandecappelle: >> >> >> On 17-03-17 08:10, Wolfgang Grandegger wrote: [snip] > # Replace the old path with the new one in all text files > while read FILE ; do > if file -b --mime-type "${FILE}" | grep -q '^text/' && [ "${FILE}" != "./usr/share/buildroot/sdk-location" ] > then > sed -i s,"${OLDPATH}","${NEWPATH}",g "${FILE}" > fi; > done < <(grep -lr "${OLDPATH}" .) > > As grep -rl is very fast; I now use the following construct handling file with names > with space corectly. BTW, is ',' is also a valid name character. Are all characters > allowed for file names? Is there a known trick? All characters except / and \0 are allowed in file names. Look at [1] for a long rant (and a number of good approaches) about this subject. Note that our scripts don't have to be portable, so we can use e.g. find -print0. But your proposal is pretty good, just needs 'read -r' insteade. So OLDPATH or NEWPATH could indeed contain a comma. You can check for that at the beginning and error out. Regards, Arnout [1] https://www.dwheeler.com/essays/filenames-in-shell.html [snip]
diff --git a/support/scripts/create-relocation-script b/support/scripts/create-relocation-script new file mode 100755 index 0000000..0ceaeb2 --- /dev/null +++ b/support/scripts/create-relocation-script @@ -0,0 +1,72 @@ +#!/bin/sh + +# Creates the script "relocate-toolchain.sh" and the "location" file in +# the buildroot toolchain (host/usr). After copying that toolchain tree +# to a new location, the script in the top directory should be executed +# to relocate the toolchain. It actually will replace the string of the +# old location with the new one in *all* text files. + +if [ "$#" -ne 1 -o ! -d "$1" ]; then + echo "Usage: $0 <buildroot-host-directory-path>" + exit 1 +fi + +# The toolchain is in buildroot's "host/usr" +TOOLCHAINDIR="$1/usr" +# We create the script in the top directory of the toolchain +SCRIPTFILE="${TOOLCHAINDIR}/relocate-toolchain.sh" +# File holding the location of the buildroot toolchain +LOCATIONFILE="share/buildroot/toolchain-location" + +echo "Creating ${SCRIPTFILE} for toolchain relocation ..." + +cat << EOF > "${SCRIPTFILE}" +#!/bin/sh +# +# Automatically generated by $0: don't edit +# +if [ "\$#" -ne 0 ]; then + echo "Run this script to relocate the buildroot toolchain at that location" + exit 1 +fi + +FILEPATH="\$(readlink -f \$0)" +NEWPATH="\$(dirname \${FILEPATH})" + +cd \${NEWPATH} +if [ ! -r ./${LOCATIONFILE} ]; then + echo "Previous location of the buildroot toolchain not found!" + exit 1 +fi +OLDPATH="\$(cat ./${LOCATIONFILE})" + +if [ \${NEWPATH} = \${OLDPATH} ]; then + echo "This buildroot toolchain has already been relocated!" + exit 0 +fi + +echo "Relocating the buildroot toolchain from \${OLDPATH} to \${NEWPATH} ..." + +# Make sure file uses the right language +export LC_ALL=C +# Replace the old path with the new one in all text files +for FILE in \$(grep -r "\${OLDPATH}" . -l ) ; do + if [ ! -h \${FILE} ] && [ -n "\$(file -b --mime-type \${FILE} | grep '^text/' )" ] && [ "\${FILE}" != "./${LOCATIONFILE}" ] + then + sed -i s,"\${OLDPATH}",\${NEWPATH},g \${FILE} + fi +done + +# Finally, we update the location file itself +sed -i s,"\${OLDPATH}",\${NEWPATH},g ./${LOCATIONFILE} + +# Check if the path substitution did work properly +if [ "\${NEWPATH}" != "\$(cat ./${LOCATIONFILE})" ]; then + echo "Something went wrong with substituting the path!" + echo "Please choose another location for your toolchain!" +fi +EOF +chmod +x "${SCRIPTFILE}" +mkdir -p $(dirname "$1/usr/${LOCATIONFILE}") +# Finally write the toolchain location +echo "$TOOLCHAINDIR" > "${TOOLCHAINDIR}/${LOCATIONFILE}"
It will create the script "relocate-toolchain.sh" in $HOST_DIR/usr/ allowing to adjust the path to the toolchain directory in all text files after it has been moved to a new location. Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> --- support/scripts/create-relocation-script | 72 ++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100755 support/scripts/create-relocation-script