Fix svn add/rm commands for release tool #27886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oandregal
requested review from
ajitbohra,
chrisvanpatten,
nerrad and
ntwb
as code owners
oandregal
added
the
[Package] Project management automation
|
Size Change: 0 B Total Size: 1.28 MB ℹ️ View Unchanged
|
|
@ockham we might want to include this fix in your PR github action PR too. |
|
I wanted to note that the |
oandregal
force-pushed
the
fix/svn-add-rm-release-commands
branch
from
fd89f33 to
b16316d
Compare
Yeah, read in some docs that macOS will silently ignore the |
|
ok, updated the docs accordingly. Unless there's anyone that wants to try this on a Linux OS before giving a 👍 I guess you need to trust me this works :D |
|
Does this mean there's a risk of breaking in MacOS? Is there a consistent way to do this? |
Not after the last change I pushed. Only linux-based OS will get the option for xargs. |
yes, that's my point, so if there's no file changes, MacOs could break (like today)? |
Ah, misunderstood your comment. Updated the testing instructions in the issue. What happens if you do this on macOS? svn checkout https://plugins.svn.wordpress.org/gutenberg/trunk test
cd test
svn st | grep '^?' | awk '{print $2}' | xargs svn addIt's the same as before (shouldn't fail), and what I understand is that it doesn't report any error precisely because the default behavior is different from linux (xargs won't run in macOS if there's no input, hence, why there's no |
|
I just tested and it seems to work without errors 👍 |
youknowriad
approved these changes
Dec 29, 2020
|
@nosolosw BSD
In my experience, having OS-specific logic will bite us when we least expect it. For example some people on macOS swap the At this point, though, it looks safe to add the |
|
From the feedback in the PR I understood |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the release tool in the use case where no files are to be added or removed for users of linux-based OS.
The commands we use to add and remove files to be committed to the WordPress svn repo for the plugin are:
However, when there are no files to add or remove, xargs still runs the svn commands, and because there are no files to add or remove, they fail and the release tools stops. On non-linux OS this works differently, reportedly on macOS, xargs won't run if it receives no input.
According to the docs, there's an option that prevents the command from being executed if no input is given,
--no-run-if-emptyor-r, but not every system supports it. To make sure it runs well in every OS we add the-roption for systems that support it.How to test
svn checkout https://plugins.svn.wordpress.org/gutenberg/trunk testcd testsvn st | grep '^?' | awk '{print $2}' | xargs -r svn add. The expected behavior is that shouldn't report any message, check that without the-rit reports an error like this one:svn st | grep '^?' | awk '{print $2}' | xargs svn add. The expected behavior is that shouldn't report any message.