search

Mbed-TLS / mbedtls-docs

Version-independent documentation for Mbed TLS
Apache License 2.0
21 stars 28 forks source link

Add script to move files to the framework #145

Open davidhorstmann-arm opened 6 months ago

davidhorstmann-arm commented 6 months ago

Pushing a small script that moves files to the framework repo from Mbed TLS, while retaining history.

This script works but may not be in a fully-polished state.

gilles-peskine-arm commented 1 month ago

I'm marking this as priority-high because we need this to move files to the framework with the history preserved. Arguably this should be in the framework repository as an essential maintainer tool (even if it'll only be essential for a few months).

gilles-peskine-arm commented 1 month ago

I've now used the script successfully. As far as I can't tell it just can't work if the destination is the framework subdirectory of an mbedtls worktree. If the source is an mbedtls worktree, the script can work, but the source worktree is disrupted because the framework submodule gets deinitialized, and the resulting history may be wrong because any file that is neither tracked nor ignored gets added.

So this script should come at least with a warning, if not enforcement, that both the source and destination should be independent worktrees, freshly checked out, that are not used for doing anything else. Preferably, the script should create these worktrees itself (as mbedtls-rewrite-branch-style does, we can reuse some of its code).

mpg commented 1 month ago

I've not yet used this script myself, but looking at its result there seems to be extra commits that I wouldn't expect to be present in the history, see https://github.com/Mbed-TLS/mbedtls-framework/pull/55#issuecomment-2404287672

mpg commented 1 month ago

I'm marking this as priority-high because we need this to move files to the framework with the history preserved. Arguably this should be in the framework repository as an essential maintainer tool (even if it'll only be essential for a few months).

Agreed. I've created https://github.com/Mbed-TLS/mbedtls-framework/issues/57 and added it to the first framework epic. (I don't care much about where the script lives, as long as it meets quality requirements.)

gilles-peskine-arm commented 1 month ago

I've not yet used this script myself, but looking at its result there seems to be extra commits that I wouldn't expect to be present in the history

We're preserving the commit history, not just the patch history. So each import branch must have the full history of the project up to the last commit that modifies one of the imported files.

The first time we did this in a pull request, GitHub showed basically all the mbedtls history as part of the pull request, because these were all new commits in the main branch of the framework. In each subsequent pull request that imports files into the framework, GitHub just shows recent commits that aren't yet in one of the imports.

There are tools that can rewrite a git branch to only retain commits that modify specific files, and construct a commit history with the same messages and the same patches for this subset of files. But you lose commit IDs (obviously), renamed files, content moved between files, etc. And these tools are harder to use. So I think we've made the right choice.

mpg commented 1 month ago

Ah OK, I had misunderstood what the script does.

There are tools that can rewrite a git branch to only retain commits that modify specific files, and construct a commit history with the same messages and the same patches for this subset of files. But you lose commit IDs (obviously), renamed files, content moved between files, etc. And these tools are harder to use. So I think we've made the right choice.

Yeah, that's what I thought the script was doing, and indeed now that you mention it that wouldn't be the right thing to do. Thanks for clearing that up.