cksource / mrgit Goto Github PK

Bisecting using mgit update is slooow. mgit checkout would help.

That would allow avoiding sth like this:

.then( () => {
	return execCommand.execute( execOptions( `git checkout ${ repository.branch }` ) );
} )
.then( ( response ) => {
	log.concat( response.logs );
} )
.then( () => {
	return execCommand.execute( execOptions( `git pull origin ${ repository.branch }` ) );
} )
.then( ( response ) => {
	log.concat( response.logs );
} )

In some configurations you may want to use a different resolver by default. It shouldn't be necessary to always pass it through CLI args.

At this moment, a user can provide the --cwd value. This does not make sense.

The cwd should indicate on the directory which contains the configuration (mgit.json) file.

cwd-resolver module looks like:

module.exports = function cwdResolver() {
	return process.cwd();
};

It should be smarter and finds the mgit.json file. If the file was not found then goes one level up and tries to find the file once again. After n attempts, it should throw an error - "The configuration file cannot be found." or something like that.

It even does make some sense now, but I'm afraid that it's just a coincidence.

We fetch before checking out so we know that the branch we want to switch exists already.

However, why then pull again?

This, surprisingly again, makes sense, because after we fetched changes, we need to update the branch we are on.

The process is safe, but:

• the --skip-fetch options makes no sense now... we're still pulling changes,
• we're requesting remote repository twice, which is 2x slower than it could be.

Just like Lerna's --ignore https://github.com/lerna/lerna#--ignore-glob

We need it to skip ckeditor5-build-* when doing some maintenance tasks on CKEditor 5 packages.

We can also implement --scope so e.g. one can do:

--scope "package-*" --ignore "package-util-*

And I've been also thinking about "groups", defined in mgit.json so one can easily pick on which packages the command should be executed without writing the globs every time.

Anyway, let's start with --ignore for now and possibly --scope.

(t/389 57e3859 ?1) p@m /www/ckeditor5/ckeditor5> mgit exec "node /www/ckeditor5/ckeditor5/scripts/switch-to-scoped-imports.js"
Execution time: 2s 27678ms.

I'd like to have repo headers (these white bars) in this case too, because I didn't even know whether it executed correctly.

In the readme I see:

Package will be installed in packages/@scope/package-name and checked out to branch develop. It will be cloned from [email protected]:organization/repository.git:

{
   "@scope/package-name": "organization/repository#develop"
}

At the save time when I added:

{
  "packages": "packages/",
  "dependencies": {
    "@ckeditor/ckeditor5-engine": "ckeditor/ckeditor5-engine"
  }
}

and run mgit bootstrap I found the project in the packages/ckeditor5-engine/ folder.

Is it a bug, mistake in readme or I don't understand something?

https://travis-ci.org/ckeditor/ckeditor5-undo/builds/193346289#L1348:

 @ckeditor/ckeditor5-utils                                           20/31 (65%) 
fatal: destination path '/home/travis/build/ckeditor/ckeditor5-undo/packages/ckeditor5-utils' already exists and is not an empty directory.

I've seen this error couple of times already.

ATM mgit highlights only the branches which:

• are different than the entry in the mgit.json,
• have unstaged files.

My point is that this pink color should help developers to focus on what belongs to the branch constellation and what does not. The most common case is that mgit.json is already configured.

Exactly the same what I reported a few days ago in ckeditor5-dev - https://github.com/ckeditor/ckeditor5-dev/issues/3.

It gives a false feeling of having the repos clean, while in fact, they have unstaged changes that need committing. They should be displayed as modified (to KISS).

cc @pomek

(t/119 33c7e0d M5) oleq@MBP ~/...ckeditor5/packages/ckeditor5-theme-lark> git st
On branch t/119
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	deleted:    theme/ckeditor5-basic-styles/code.css
	deleted:    theme/ckeditor5-block-quote/blockquote.css
	deleted:    theme/ckeditor5-image/image.css
	deleted:    theme/ckeditor5-image/imagecaption.css
	deleted:    theme/ckeditor5-image/imagestyle.css

oleq@MBP ~/CK/5/ckeditor5> mgit st
Collecting statuses...
┌──────────────────┬─────────┬─────────┬────────┐
│ Package          │ Branch  │ Commit  │ Status │
├──────────────────┼─────────┼─────────┼────────┤
...
│ theme-lark       │ ! t/119 │ 33c7e0d │        │
...
└──────────────────┴─────────┴─────────┴────────┘

mgit update set versions in packages folder to the versions defined in mgit.json file. But the opposite command would be also needed, to set versions in mgit.json to hashes in the packages.

I propose the mgit save-hashes command which goes into each repository check the current hash and add #hash to each entry in mgit.json. It's very useful if the project uses fixed versions of dependencies and update them manually.

Since I always forget to change mgit.json/package.json while I develop and whenever I ask someone to check my branch I need to give him the whole list of branches I realized that the mgit --watch tool would be very useful. It would watch all subbranches and whenever branch changes its saves it to the mgit.json or package.json (see #29).

 #  @ckeditor/ckeditor5-theme-lark                                       16/20 (80%)
Already on 'master'
Your branch is up-to-date with 'origin/master'.
From github.com:ckeditor/ckeditor5-theme-lark
 * branch            master     -> FETCH_HEAD
Current branch master is up to date.

{ logs:
   { error: [ 'From github.com:ckeditor/ckeditor5-utils\n * branch            master     -> FETCH_HEAD\n\nIt seems that there is already a rebase-apply directory, and\nI wonder if you are in the middle of another rebase.  If that is the\ncase, please try\n\tgit rebase (--continue | --abort | --skip)\nIf that is not the case, please\n\trm -fr "/workspace/ckeditor5/packages/ckeditor5-utils/.git/rebase-apply"\nand run me again.  I am stopping in case you still have something\nvaluable there.' ],
     info: [] } }
TypeError: msg.trim is not a function
    at Object.log (/workspace/mgit2/lib/utils/log.js:29:15)
    at Object.error (/workspace/mgit2/lib/utils/log.js:24:18)
    at execCommand.execute.then.then.then.then.then.then.then.catch (/workspace/mgit2/lib/commands/update.js:93:10)
    at process._tickCallback (internal/process/next_tick.js:103:7)

There are two issues here:

1. That I needed to modify this code to get any meaningful error because, by default, you only see a short info about unhandled promise.
2. That our git status -s doesn't catch this particular information. However, if the error which I can see inside catch() in update.js after adding console.log() there, was properly logged to the console it would be enough. After all, the situation I have (with unfinished rebase) is not very common.

It would be helpful not to define the same data twice.

For instance: if my project use fixed hashes, these hashes should be stored in both package.json (to let developers who won't contribute to dependencies use simple npm install) and in mgit.json (for developers who will contribute to dependencies to have proper versions by default).

I understand that there are cases where two separate configurations need to be defined, but for some cases, one is better. My proposal is that if mgit.json is not defined then mgit use package.json.

https://github.com/ckeditor/ckeditor5-dev/issues/2 introduced tools for generating the changelog and creating the release.

Mgit2 can use it.

mgit -v should return version. That's it.

At this moment, repository resolver returns two things:

1. repository URL,
2. branch to clone (most common it is master).

It is enough for case - mgit reads the mgit.json file.

But, if bootstrap command is called with option --recursive, the package.json file of cloned package is also read. Mgit will install all required dependencies by cloned package.

Resolver does not allow to change a directory for an installed package. Mgit will always install in the directory: packages/{ packageName }. Would be nice to have configurable the directory for packages required by other packages.

Repository resolver could return three things:

1. repository URL,
2. branch to clone,
3. the directory name.

It allows removing the npm scope from package name. It means, a package @organization/some-package would be installed under packages/some-package instead of packages/@organization/some-package as it is now.

The first 6 characters are enough to identify a commit, so to not bloat the mgit.json too much it would be nice to save only them.

In package.json we say this is MIT while in every file we point to a non-existing LICENSE.md

We should do like here: https://github.com/benderjs/benderjs/blob/master/lib/browsers.js

I see across the code the name parameters. Function definition have parameters. A function is called with arguments or it can have options.

• Some code snippets are incorrectly rendered on npm: https://www.npmjs.com/package/mgit2
• ToC is missing (take https://github.com/ckeditor/ckeditor5 for instance)
• Mention other projects using it?

Happens in ckeditor5 now due to #23. Related to #21.

First I thought that we have two options:

• use the resolver to find directory name (like the other commands do, so part of that logic could be extracted to a util),
• run the command on directories found in packages/, not on dependencies found in mgit.json.

The latter sounds simpler, but it'd be incorrect. Since a resolver can return any a/b/c directory name (e.g. a scoped dir like @org/repo), then a command would fail in such a case.

BTW, the next step would be to add --recursive option to the exec command too. It should have the same behaviour as in other commands. We start with what is defined in mgit.json and if --recursive is defined we go deeper.

Not very clear now:

execCommand.execute( execOptions( `git checkout ${ repository.branch }` ) );

After installing mgit via npm (npm install mgit2), I've checked out of curiosity the contents of its directory and I saw tests dir, .travis.yml, gulpfile.js etc. These files are needed only for developing mgit, not for using it.

Maybe it could be fine to just whitelist needed file (index.js and lib/ dir) using files field in package.json (npm always include package.json, readme, changelog and license anyway)? Or just blacklist all the files using .npmignore?

Mgit allows installing packages based on specify branch, tag or even commit id.

Unfortunately, mgit cannot update the package by checking out to specify commit id.

Steps to reproduce:

1. Check out a package to master
2. Add some commit id to mgit.json as a branch
3. Execute mgit update.

Expected behavior:

Modified package will be checked out to a specify commit.

Actual behavior:

Console logs:

(node:61534) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: msg.trim is not a function

Should pretty-print the statuses of all repos.

We need to figure out which information are the most important and a nice way to print them (some sort of a table?).

Right now if you clone repos recursively and try to do mgit exec only the packages listed in mgit.json will be included.

So either they should be added there automatically (makes some sense) or exec and update tasks should read packages by scanning the packages/ directory rather than based on mgit.json.

Let's assume that we're developing some package and clone it directly to the packages directory.
Now when running npm run test in CKE5, this package will be tested. But mgit st won't show that something is wrong.

PS: I'm not sure whether this should be done on the mgit side or the CKE5 side.

Changes are pulled regardless of the fact that there are uncommitted changes in a certain repo.

Do e.g. this:

cd packages/ckeditor5-typing
git reset --hard HEAD~
touch x

cd ../..
mgit update

Output:

 ckeditor5-typing
Package "ckeditor5-typing" has uncommitted changes. Aborted.

But the changes were pulled anyway.

Currently, it's always a Git+ssh URL which is not going to work on Travis. The repository resolver should return a whole URL, not just <org>/<repoName>. I even documented it like this in the README:

By default, mgit2 uses a simple repository resolver which returns git URL for a package name based on dependencies specified in mgit.json.

There is a save-hashes option what is really nice but It would be useful to have save-branches option. Hashes are nice if you want to bump packages at the end of your work, but when you are in the middle of something you may want to push set of branches.

WDYT?

I can see that the bootstrap and update commands are very similar. I wanted to have mgit bootstrap to follow Lerna's naming, but it doesn't make sense in case of mgit. Mgit clones repos and pulls changes, which is a bit different job. Let's have a single mgit pull command for that. It will be used to init the repositories and later to pull changes to them.

WDYT?

After calling the bootstrap command, required packages are installed in packages/ directory.

Tools like Webpack won't search the package in the custom directory. From Webpack point of view, packages would be installed in node_modules directory.

Bootstrap command after installation a package, could create a symlink:

|-- packages
|  |-- some-dependency
|  `-- @scope
|     `-- other-dependency
|-- node_modules
|  |-- some-dependency (symlink to /packages/some-dependency)
|  |-- @scope
|     `-- other-dependency (symlink to /packages/@scope/other-dependency)
|-- mgit.json

In order to improve or fixing bugs in existing command, we need to create the tests.

Right now, if you call mgit bootstrap and then mgit bootstrap --recursive, the second call will do nothing, even if there are repository to be cloned.

This happens because the bootstrap command aborts immediately, without checking what else it could clone.

PS. After merging bootstrap into update, the same issue will apply to update.

I've got following error after git update:

(node:11920) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: msg.trim is not a function

I checked packages and one of them was on the other branch that needs to merge after git pull

 #  @ckeditor/ckeditor5-utils                                            16/18 (89%) 
Your configuration specifies to merge with the ref 'refs/heads/t/116'
from the remote, but no such ref was fetched.

So the error from git update could be similar to provide a useful insight into the problem.

A few days ago, in the CKE5 we removed guppy-pre-commit which was used for linting changes before committing (ckeditor/ckeditor5#597).

Next step is to remove Gulp permanently – ckeditor/ckeditor5#473. We could do the same for Mgit.

I've tried to work with exact commit hash but mgit2 failed.

It would be nice if mgit2 would accept also a commit hash and id when resolving dependencies:

{
	"packages": "packages/",
	"dependencies": {
		"ckeditor5-engine": "https://github.com/ckeditor/ckeditor5-engine.git#890e9bb9a03677f87ea8458222d3ec995b5582c6"
	}
}

or

{
	"packages": "packages/",
	"dependencies": {
		"ckeditor5-engine": "https://github.com/ckeditor/ckeditor5-engine.git#890e9bb"
	}
}

We can either use ASCII (e.g. precede the name with # <name>) or use different colors. Apparently, Travis ignores inverse().

In order to follow Lerna's convention.

Execution time: 2s 27678ms.

27678ms is a bit long itself :D

(By repo title bar I mean the white rectangle.)

If you use a command, you need to call its methods (beforeExecute, execute and afterExecute) manually.

I may be the author of this, but I was confused whether the tool failed gracefully or heavily. If we catch an issue, it shouldn't be that verbose. We just need the message to be logged.