Make WordPress Core

Opened 19 years ago

Closed 19 years ago

Last modified 7 weeks ago

#3858 closed defect (bug) (duplicate)

unclosed apply_filters() in /wp-includes/feed.php

Reported by: znuff's profile Znuff Owned by:
Milestone: Priority: high
Severity: blocker Version: 2.2
Component: General Keywords: has-patch
Focuses: Cc:

Description

In lines 180 and 195 there is a missing ).

Change History (10)

#1 @Znuff
19 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

damn connection!@$!#@$

#2 @Nazgul
19 years ago

  • Keywords haspatch removed
  • Milestone 2.2 deleted

Duplicate of #3859.

This ticket was mentioned in PR #11064 on WordPress/wordpress-develop by @dmsnell.


2 months ago
#3

  • Keywords has-patch added

Exploration to restore history of deleted files.

## Status

This does not appear ready for merge, as it looks like the way I copied changes from the Gutenberg sync commits is committing the built artifact of things like CSS files and not the source.

  • src/wp-admin/css/color-picker.css

    diff --git a/src/wp-admin/css/color-picker.css b/src/wp-admin/css/color-picker.css
    index 2e038353b7..eb384ea370 100644
    a b  
    117117
    118118.iris-picker .ui-square-handle:focus,
    119119.iris-picker .iris-strip .ui-slider-handle:focus {
    120         border-color: var(--wp-admin-theme-color, #3858e9);
     120        border-color: #3582c4;
    121121        border-style: solid;
    122         box-shadow: 0 0 0 var(--wp-admin-border-width-focus, 1.5px) var(--wp-admin-theme-color, #3858e9);
     122        box-shadow: 0 0 0 1px #3582c4;
    123123        outline: 2px solid transparent;
    124124}

In other places it looks like some changes to Core were reverted; I will verify what happened, as some changes may have been applied to wordpress-develop and might be inadvertently reverted by the build change, but without making it clear to see that the changes were wiped out.

  • src/wp-admin/admin-header.php

    diff --git a/src/wp-admin/admin-header.php b/src/wp-admin/admin-header.php
    index e1e9ba0f65..ea0245fdb1 100644
    a b wp_enqueue_script( 'svg-painter' ); 
    101101
    102102$admin_body_class = preg_replace( '/[^a-z0-9_-]+/i', '-', $hook_suffix );
    103103?>
    104 <script>
     104<script type="text/javascript">

---

Note:

  • This _must_ merge as a non-squashed commit, because the commit parent must be accessible to preserve history.
  • This needs to also come through in the svn-to-git conversion. If the merge is lost in that process then it won’t matter if this PR restores the history, because it will be lost when back-writing from the Subversion source.

## Testing

Here is a sequence of events that simulates this operation. The script creates an SVN repo, adds some files and makes a few meaningless commits, then deletes a test.txt file.

There is a git svn mirror tracking the SVN repo.

From the SVN side, a new branch is created to restore the files. It is forked from before they were deleted and then the test.txt file is modified in a neutral way so that it will create a merge conflict. This is important because otherwise svn and git will automatically accept the deleted files as the truth.

That branch is merged in which tracks the version history for the files, because it maintains metadata pointing to the commits before they were deleted.

On the git svn side though it’s critical to first git svn fetch --all to retrieve the new branch (otherwise it will not have the metadata and therefore linearize the merge), and then to run git rebase --merge --rebase-merges so that it avoid linearizing the merge.

https://github.com/user-attachments/assets/ccc5cd1b-2e61-4895-ac1f-4450299bffeb

#!/usr/bin/env bash

ROOT=$(pwd)
REPO=file://${ROOT}/svn-repo

printf "\e[90mStep \e[33m1\e[90m — create SVN repo\e[m\n"
svnadmin create svn-repo
svn mkdir ${REPO}/trunk -m "Create trunk"
svn mkdir ${REPO}/branches -m "Create branches"

printf "\e[90mStep \e[33m2\e[90m — load content into SVN checkout\e[m\n"
svn co ${REPO} svn-checkout
pushd svn-checkout/trunk
svn up ..
echo "Testing instructions" > test.txt
echo "1 2 3 4" > data.dat
svn add test.txt data.dat
svn commit -m "Initial commit"
popd

printf "\e[90mStep \e[33m3\e[90m — establish \e[2mgit\e[0;90m mirror\e[m\n"
git svn clone -T trunk -b branches ${REPO} git-repo

printf "\e[90mStep \e[33m4\e[90m — make noisy change\e[m\n"
pushd svn-checkout/trunk
echo "\n\n1. look at it" >> test.txt
echo " 5" >> data.dat
svn commit -m 'Expand testing'

echo "<?php\n\ndie();" > feature.php
svn add feature.php
svn commit -m 'Add feature skeleton'
svn up
popd

pushd git-repo
git svn rebase 
popd

printf "\e[90mStep \e[33m5\e[90m — remove test.txt\e[m\n"
pushd svn-checkout/trunk
svn rm test.txt
echo "/test.txt" > .gitignore
svn add .gitignore
svn commit -m 'Remote tests'
popd

printf "\e[90mStep \e[33m6\e[90m — make more noisy commits\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(1);" > feature.php
svn commit -m 'Die abnormally'

echo "Are you lost?" > directions.html
svn add directions.html
svn commit -m 'Add directions'
svn up
popd

pushd git-repo
git svn rebase 
printf "\n\n\e[90mState of the git repo after deleting the tests\e[m\n"
git log --graph --topo-order --oneline
popd

printf "\e[90mStep \e[33m7\e[90m — Branch before removal to restore\e[m\n"
pushd svn-checkout/trunk
svn copy -r5 ${REPO}/trunk ${REPO}/branches/restore-tests -m 'Restore tests (branch from f5 before removal)'
svn up ../
popd

pushd svn-checkout/branches/restore-tests
echo "\n" >> test.txt
echo "" > .gitignore
svn commit -m 'Introduce merge conflict to force restoration'
svn up ../
popd

pushd git-repo
git svn rebase 
popd

printf "\e[90mStep \e[33m8\e[90m — Noisy commit\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(0);" > feature.php
svn commit -m 'Die normally'
svn up
popd

pushd git-repo
git svn rebase 
printf "\n\n\e[90mState of the git repo after noisy commits\e[m\n"
git log --graph --topo-order --oneline
popd

printf "\e[90mStep \e[33m9\e[90m — Merge branch and restore tests\e[m\n"
pushd svn-checkout/trunk
svn merge ${REPO}/branches/restore-tests --accept postpone
ls ../branches/restore-tests
cp ../branches/restore-tests/test.txt .
svn resolve --accept working test.txt
svn add test.txt
svn commit -m 'Merge and restore tests'
svn up ../
popd

pushd git-repo
git svn fetch --all
git svn rebase
printf "\n\n\e[90mState of the git repo after the merge\e[m\n"
git log --graph --topo-order --oneline
popd

@dmsnell commented on PR #11064:


2 months ago
#4

Changed branch HEAD from 29b508c2feaba4a305ec8c969395b923e16827ef to 4220ecc44fd76b405ca4f39092f32a439f9a0230

This is to explore a new process to rebuild the branch. Details to come later, but I’m going to try and merge trunk into the branch at each sync-commit along the way.

@dmsnell commented on PR #11064:


2 months ago
#5

Recreated branch from previous 59e0a279175edddf034304821f0430e3671d62ad to new 49093f1b4428b6bf0d4f6bfdc97a4135207bc4b1

This time the creation was automated with the following script:

https://gist.github.com/dmsnell/c6f068e7f6028bacbfcb810d2fe726c2

@jonsurrell commented on PR #11064:


2 months ago
#6

I picked a file that I expected to have a long history. The paragraph block.json. If I start at the HEAD of this branch and navigate to its earliest history, I reach https://github.com/dmsnell/wordpress-develop/commit/f7d617cfb8e71ead2ab589011bffad5a22f35d3c. If I start at an arbitrary place in Trac before the break in history (6.0) and navigate back, it traverses merges and ultimately arrives at the corresponding commit: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/blocks/paragraph/block.json?rev=48262

That file doesn't exist at all in `trunk` right now.

It's a good indicator that this branch is an adequate solution to fixing history. 👍

---

I'm certainly missing some background, but I'd like to understand how this will work in practice.

Although they were deleted in trunk, this patch, when applied as a merge commit will provide two parents which will allow any and all git tooling to reconstruct the history of the files without any special options or flags.

Ultimately, things need to happen in subversion and then propagate to git mirrors from there. Work is committed with svn commit. Will this rely on git svn dcommit or something along those lines? Will a branch need to live on subversion permanently to hold these commits as a bridge from the current trunk revision back to the history-restoring point?

@dmsnell commented on PR #11064:


8 weeks ago
#7

Ultimately, things need to happen in subversion and then propagate to git mirrors from there. Work is committed with svn commit. Will this rely on git svn dcommit or something along those lines? Will a branch need to live on subversion permanently to hold these commits as a bridge from the current trunk revision back to the history-restoring point?

Thanks @sirreal for your review and the question. This is something I wanted to ensure was simulated before merging, which you can see if you use the attached simulator script.

To the best of my knowledge, when subversion creates the merge with svn merge, then git svn rebase will create a merge commit, as long as the commits from the branch have been previously fetched with git svn fetch. We already have the branch created on the git side, so I am confident that as we commit the changes into the svn branch, they will carry over into git as well.

The final commit will be the merge commit itself, and since both of its parent’s will already by synced, we should have no problem seeing it created in git

My plan was to recreate the commits in svn first rather than using git svn dcommit but I believe we could use that approach too.

@dmsnell commented on PR #11064:


8 weeks ago
#8

Recreated branch from 45aafcc1882b70349839aee9e244c0ca5afda312 to af434d639d93f2c3eeecbee9e8d164d92111326b

@jonsurrell commented on PR #11064:


8 weeks ago
#9

The commits here in GitHub seem to be correct and will likely resolve the issue of broken and lost history that this seeks to address.

This isn't a simple commit, much of the interesting work here is the process of how the commits are actually made. I've reviewed the process with @dmsnell and it seems to work well.

@dmsnell commented on PR #11064:


7 weeks ago
#10

Merged in [62143]
08be2764e3f46f2729e5a4f0af34a6d3150d640b

Note: See TracTickets for help on using tickets.