Changeset View
Standalone View
usr.sbin/freebsd-update/freebsd-update.sh
| Show First 20 Lines • Show All 2,897 Lines • ▼ Show 20 Lines | backup_kernel () { | ||||
| # Re-enable patchname expansion. | # Re-enable patchname expansion. | ||||
| set +f | set +f | ||||
| } | } | ||||
| # Install new files | # Install new files | ||||
| install_from_index () { | install_from_index () { | ||||
| # First pass: Do everything apart from setting file flags. We | # First pass: Do everything apart from setting file flags. We | ||||
| # can't set flags yet, because schg inhibits hard linking. | # can't set flags yet, because schg inhibits hard linking. | ||||
emaste: Oh I guess -e && -d is redundant, will drop -e
| |||||
| sort -k 1,1 -t '|' $1 | | sort -k 1,1 -t '|' $1 | | ||||
| tr '|' ' ' | | tr '|' ' ' | | ||||
| while read FPATH TYPE OWNER GROUP PERM FLAGS HASH LINK; do | while read FPATH TYPE OWNER GROUP PERM FLAGS HASH LINK; do | ||||
| # A file may change to a directory on upgrade (PR273661). | |||||
| # If that happens rm the file first so that install can create | |||||
| # the directory in its place. | |||||
| if [ -e "${BASEDIR}/${FPATH}" ]; then | |||||
| if [ ${TYPE} = d ] && ! [ -d "${BASEDIR}/${FPATH}" ]; then | |||||
| rm -f -- "${BASEDIR}/${FPATH}" | |||||
dimUnsubmitted Not Done Inline ActionsThis could be shortened as: if [ -e "${BASEDIR}/${FPATH}" ] && [ ${TYPE} = d ] && ! [ -d "${BASEDIR}/${FPATH}" ]; thensince && short-circuits. But it's mostly a matter of preference. dim: This could be shortened as:
```
if [ -e "${BASEDIR}/${FPATH}" ] && [ ${TYPE} = d ] && ! [ -d… | |||||
khorben_defora.orgUnsubmitted Not Done Inline ActionsOr maybe even: if [ -e "${BASEDIR}/${FPATH}" -a ${TYPE} = d -a ! -d "${BASEDIR}/${FPATH}" ]; then(it reduces the amount of calls to test through [, although it is a built-in in sh anyway) khorben_defora.org: Or maybe even:
```
if [ -e "${BASEDIR}/${FPATH}" -a ${TYPE} = d -a ! -d "${BASEDIR}/${FPATH}"… | |||||
emasteAuthorUnsubmitted Done Inline ActionsThat doesn't have the benefit of short-circuiting though (and [ is a built-in anyhow, as you say). In any case that change is already committed and is independent of this review :) emaste: That doesn't have the benefit of short-circuiting though (and `[` is a built-in anyhow, as you… | |||||
| fi | |||||
| fi | |||||
| case ${TYPE} in | case ${TYPE} in | ||||
| d) | d) | ||||
| # Create a directory | # Create a directory | ||||
| install -d -o ${OWNER} -g ${GROUP} \ | install -d -o ${OWNER} -g ${GROUP} \ | ||||
| -m ${PERM} ${BASEDIR}/${FPATH} | -m ${PERM} ${BASEDIR}/${FPATH} | ||||
| ;; | ;; | ||||
| f) | f) | ||||
| if [ -z "${LINK}" ]; then | if [ -z "${LINK}" ]; then | ||||
| ▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines | d) | ||||
| rmdir ${BASEDIR}/${FPATH} | rmdir ${BASEDIR}/${FPATH} | ||||
| ;; | ;; | ||||
| f) | f) | ||||
| rm ${BASEDIR}/${FPATH} | rm ${BASEDIR}/${FPATH} | ||||
| ;; | ;; | ||||
| L) | L) | ||||
| rm ${BASEDIR}/${FPATH} | rm ${BASEDIR}/${FPATH} | ||||
| ;; | ;; | ||||
| esac | esac | ||||
Not Done Inline ActionsIf you are deleting directories in dir_conflict using rm -rf I think it is reasonable to delete these offending files with rm -f too, and in that case it is not necessary to do the if [ -e, since rm will shut up and not fail in such cases. dim: If you are deleting directories in `dir_conflict` using `rm -rf` I think it is reasonable to… | |||||
Done Inline ActionsI've switched this to [ -f "${BASEDIR}/${FPATH} ] and [ -L "${BASEDIR}/${FPATH} ] locally now, because we still get an error message: $ touch not-a-dir emaste: I've switched this to `[ -f "${BASEDIR}/${FPATH} ]` and `[ -L "${BASEDIR}/${FPATH} ]` locally… | |||||
Not Done Inline ActionsTrue, I did not think of that. khorben_defora.org: True, I did not think of that. | |||||
| done < killfiles | done < killfiles | ||||
| # Clean up | # Clean up | ||||
| rm newfiles killfiles | rm newfiles killfiles | ||||
| } | } | ||||
| # Install new files, delete old files, and update generated files | # Install new files, delete old files, and update generated files | ||||
| install_files () { | install_files () { | ||||
| ▲ Show 20 Lines • Show All 543 Lines • Show Last 20 Lines | |||||
Oh I guess -e && -d is redundant, will drop -e