[PATCH] D41393: Allow to apply cherry-picks when building Docker images.

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.


================
Comment at: utils/docker/scripts/build_install_llvm.sh:185
+# Sort cherrypicks and remove duplicates.
+CHERRYPICKS="$(echo "$CHERRYPICKS" | xargs -n1 | sort | uniq | xargs)"
+
----------------
ioeric wrote:
> I'd do the sorting in `apply_cherrypicks`. 
Why not here? `apply_cherrypicks` is called multiple times, sorting outside of it seems natural.
Maybe we should define `apply_cherrypicks` after we do the sorting or add a comment to make it clear that it always applies cherrypicks in a sorted order instead?


================
Comment at: utils/docker/scripts/build_install_llvm.sh:207
+
+  apply_cherrypicks "$CLANG_BUILD_DIR/src/$LLVM_PROJECT"
 done
----------------
ioeric wrote:
> IIUC,`$CHERRYPICKS` is a list of cherries for all llvm sub-projects, and we try applying them on each project, even if some of them might not apply for certain projects. Is this safe with svn? Might worth a comment. 
Fortunately we're good here, `svn diff -c` provides empty patches for revisions that don't touch specific repositories so we end up with a no-op.
Will add a comment.


https://reviews.llvm.org/D41393