media icon indicating copy to clipboard operation
media copied to clipboard

Migrating from google/ExoPlayer repo

Open stevemayhew opened this issue 2 years ago • 9 comments

Is there a document, scripts or any workflow tips you can share for migrating code (pull request branches in particular) from the legacy ExoPlayer?

I have a number of open pull requests as well as a code base that is r2.16 based I want to migrate and would like to keep history and/or links.

stevemayhew avatar Dec 07 '22 18:12 stevemayhew

Here's a migration guide including a script: https://developer.android.com/guide/topics/media/media3/getting-started/migration-guide

moneytoo avatar Dec 08 '22 20:12 moneytoo

Thanks, that will work for the application... But we have changes to ExoPlayer itself, library/core, hls, extractor, etc.

Most are shared as pull requests (or soon to be). Looks like internally you migrated in place on a branch in the old ExoPlayer git, then maybe cloned to AndroidX?

stevemayhew avatar Dec 09 '22 05:12 stevemayhew

What do you envision would that be? The mentioned script has all the mappings (although, I just realized it has not been updated since the last release. I think it should be still the same mappings but I need to verify). However, the script has the mappings and you can display them by using the -p, -c or -d flags. We can also publish the mappings in another format if that would be helpful. I wonder what format would be best for your use case.

Internally, moving to the new media3 packages was a manual process in the library code and all Google apps that had dependencies.

marcbaechinger avatar Dec 09 '22 10:12 marcbaechinger

Not sure what that "migration script" is really intended to do, but for me it seemed to make copies of most or all repo files in place, adding two single quotes to the end of the filename. Essentially my entire repo is a total mess now! Thanks @marcbaechinger

bubenheimer avatar Mar 31 '23 19:03 bubenheimer

I'm sorry to hear about the script creating inconvenience for you. Please accept my apologies. That's certainly disastrous as the script attempts to help developers migrating instead of messing up their projects. :(

If you want to give me some more details about the operating system under which you experienced the script creating a mess that would be great. I'd like to politely ask you for more details so that I can try to reproduce the behavior.

You are correct, that the script is creating an in place copy of the files and replaces package names (and potentially class names) to match the new package structure. The script uses sed with inplace replacement, so without any further investigation it's a bit unclear to me why the output path changes.

If you can give me some details on how you called the script on what operating system, I can try to repro and see whether we can fix this.

Please accept my apologies for the faulty behavior of the script and the inconvenience created by this.

marcbaechinger avatar Apr 04 '23 15:04 marcbaechinger

I just ran the script on Intel Mac, I don't think there is anything particularly special about my shell. Mac uses zsh by default.

Even without the renaming problems the script does not seem to help at all. In the sea of copied, mangled files there were one or two that had actual changes in it, but they did not change classes & packages from exoplayer over to Media3. One example of what it did do was to pointlessly rename my Exoplayer StyledPlayerView to Exoplayer PlayerView, so it seemed to make the code less ready for migration than before. Maybe it meant to change it to Media3 PlayerView, but skipped the Media3 part.

I did not run the script with any special options, just '-m'.

In addition, the script should never touch anything that it has no reason to modify in the first place. Instead it made copies of every single file, whether it's source code or some other random file. The '-l' option just prints all files in the folder structure, which is not useful. What would be useful is listing changed files from a test run that does not actually physically modify anything.

You gotta pull this script.

bubenheimer avatar Apr 04 '23 15:04 bubenheimer

Also, the mappings documentation is quite flawed, look at the "media3 class names" in this section, for example: https://developer.android.com/guide/topics/media/exoplayer/mappings#class-renamings

These migration aids are so bad that it's probably faster to figure stuff out on your own than to use scripts or docs.

bubenheimer avatar Apr 04 '23 15:04 bubenheimer

I have corrected the listing with the class renamings. Thanks for reporting this and the problems with the script. This is appreciated.

I changed the script that now uses a more conservative approach and doesn't exhibit the described problems. I will sync this with the dev-v2 branch and then cherry-pick into the release branch asap.

marcbaechinger avatar Apr 04 '23 21:04 marcbaechinger

Thanks @marcbaechinger I imagine this migration-script is rather unglamorous to maintain ;-).

The correct thing to do is migrate in-place. None of the moves conflict with existing ExoPlayer files so it is possible (I would hope ;-), with possibly a bit of manual effort ) to have a Media3 branch/tree in place in the same repository as a legacy ExoPlayer. This is our intention for migrating our ExoPlayer tree to AndroidX while maintaining history. We have a relatively small number of changes to ExoPlayer core libraries that have incomplete pull requests.

The package renaming (PACKAGE_MAPPINGS) for fixing imports looks wrong to me, the src and destination are the same:

$ ./media3-migration.sh  -p
com.google.android.exoplayer2                           com.google.android.exoplayer2 
com.google.android.exoplayer2.analytics                 com.google.android.exoplayer2.analytics
com.google.android.exoplayer2.audio                     com.google.android.exoplayer2.audio
com.google.android.exoplayer2.castdemo                  com.google.android.exoplayer2.castdemo
com.google.android.exoplayer2.database                  com.google.android.exoplayer2.database
...

I would expect they would match: https://developer.android.com/media/media3/exoplayer/mappings#package-mappings

This version does: https://github.com/google/ExoPlayer/pull/11377

stevemayhew avatar Feb 26 '24 22:02 stevemayhew