Handle empty issue list more gently + avoid fetching the git remote directly by wilzbach · Pull Request #217 · dlang/tools

wilzbach · 2017-02-17T23:52:23Z

When there are no issues in the git log, the bugzilla query is empty and thus all issues get loaded. This is fix that solves the problem by simply returning earlier and thus not sending the Bugzilla query.
Also there's also the category "VisualD" on Bugzilla into which on might run as well (even though it's currently not on list of parsed repos).

Moreover, as dlang.org changelog scheme is without "v", there's another simple fix that prints the previous version with the "v" prefix.

CyberShadow · 2017-02-18T01:16:14Z

Darn, looking at it more closely it looks like the remote name is both assumed in the program to be upstream, and it is assumed that it is specified by the user on the command-line as upstream/<branch>. So if we are to fix this, we would need to be OK with breaking existing usages of the script. Would this be an issue if the changelog will be automatically generated from now on?

CyberShadow · 2017-02-18T01:20:31Z

changed.d

-        auto cmd = ["git", "-C", repo, "fetch", "upstream", "--tags"];
+        // ensure that changed_upstream exists (we ignore failures)
+        auto cmd = ["git", "-C", repo, "remote", "add", "changed_upstream", "https://github.com/dlang/" ~ repo.baseName];
+        execute(cmd);


We can fetch directly without adding a remote:

cmd = ["git", "-C", repo, "fetch", "--tags", "https://github.com/dlang/" ~ repo.baseName, "+refs/heads/*:refs/remotes/upstream/*", "--tags"];

This avoids adding a remote and preserves compatibility with the existing command-line usage (specifying upstream/ before branch names), though it will overwrite the tracking refs of whatever remote the user has that's named upstream, assuming one exists.

Git namespaces seem to allow fixing everything at once, though.

CyberShadow · 2017-02-18T01:21:45Z

Rename changed.d branch to changed_upstream

I think you meant remote, not branch

CyberShadow · 2017-02-18T02:11:06Z

Add environment["GIT_NAMESPACE"] = "changed"; at the top of main
Avoid adding a remote by fetching directly (see code example in inline comment above)
~~Fix the commit message maybe~~ (will be obsoleted by changes above)

With that and @MartinNowak's approval, I think this will be good to go.

wilzbach · 2017-02-26T01:52:33Z

Add environment["GIT_NAMESPACE"] = "changed"; at the top of main

Can't we do this directly in the pipeProcess command?

Avoid adding a remote by fetching directly (see code example in inline comment above)

Nice!

With that and @MartinNowak's approval, I think this will be good to go.

Rebased and waiting for approval.

CyberShadow · 2017-02-26T02:39:23Z

Can't we do this directly in the pipeProcess command?

Sure, but you'll need to do this for all git invocations.

CyberShadow · 2017-02-26T02:40:10Z

changed.d

+        auto p = pipeProcess(cmd, Redirect.stdout, ["GIT_NAMESPACE": "changed"]);
        enforce(wait(p.pid) == 0, "Failed to execute '%(%s %)'.".format(cmd));

        cmd = ["git", "-C", repo, "log", revRange];


This needs to have the GIT_NAMESPACE thing too, otherwise it won't use the correct refs. Right?

CyberShadow · 2017-02-26T02:41:39Z

If you want to avoid changing the environment, there's also --namespace=<name>. But the environment variable has the advantage that it will fall back to something that will probably still work sensibly for older git versions, that don't support namespaces.

Use a git NAMESPACE to avoid interference with the user's refs.

wilzbach · 2017-02-26T10:16:01Z

Okay so I gave this another try locally with:

./changed.d "v2.072.2..upstream/stable

And upstream isn't appearing as remote:

> git remote -v
origin	git@github.com:dlang/dmd.git (fetch)
origin	no_push (push)

(I set no_push manually to avoid any possible errors caused by this script)

However, within the list of remote branches the just fetched branches appear:

> git branch -r
  origin/2.061
  origin/2.062
  origin/2.063
  origin/2.064
  origin/2.065
  origin/2.066
  origin/2.067
  origin/HEAD -> origin/master
  origin/MartinNowak-patch-1
  origin/dmd-1.x
  origin/master
  origin/newCTFE
  origin/rainers-patch-1
  origin/revert-3761-issue13116
  origin/revert-5744-refactor_attrib
  origin/revert-6217-ddmd-symlink
  origin/stable
  upstream/2.061
  upstream/2.062
  upstream/2.063
  upstream/2.064
  upstream/2.065
  upstream/2.066
  upstream/2.067
  upstream/MartinNowak-patch-1
  upstream/dmd-1.x
  upstream/master
  upstream/newCTFE
  upstream/rainers-patch-1
  upstream/revert-3761-issue13116
  upstream/revert-5744-refactor_attrib
  upstream/revert-6217-ddmd-symlink
  upstream/stable

I also tried it manually - with the same effect:

GIT_NAMESPACE="changed" git fetch --tags https://github.com/dlang/dmd '+refs/heads/*:refs/remotes/upstream/*'                                                                                          
From https://github.com/dlang/dmd
 * [new branch]          2.061                       -> upstream/2.061
 * [new branch]          2.062                       -> upstream/2.062
 * [new branch]          2.063                       -> upstream/2.063
 * [new branch]          2.064                       -> upstream/2.064
 * [new branch]          2.065                       -> upstream/2.065
 * [new branch]          2.066                       -> upstream/2.066
 * [new branch]          2.067                       -> upstream/2.067
 * [new branch]          MartinNowak-patch-1         -> upstream/MartinNowak-patch-1
 * [new branch]          dmd-1.x                     -> upstream/dmd-1.x
 * [new branch]          master                      -> upstream/master
 * [new branch]          newCTFE                     -> upstream/newCTFE
 * [new branch]          rainers-patch-1             -> upstream/rainers-patch-1
 * [new branch]          revert-3761-issue13116      -> upstream/revert-3761-issue13116
 * [new branch]          revert-5744-refactor_attrib -> upstream/revert-5744-refactor_attrib
 * [new branch]          revert-6217-ddmd-symlink    -> upstream/revert-6217-ddmd-symlink
 * [new branch]          stable                      -> upstream/stable

I am not sure whether this is a potential issue for the few people that use changed.d?

CyberShadow · 2017-02-26T10:30:28Z

What the heck. I can't get it to work either.

CyberShadow · 2017-02-26T10:31:23Z

Git namespaces is only intended to work with remote repos

I'm sorry, I must have confused it with something else :(

CyberShadow · 2017-02-26T10:32:28Z

OK, I say to just drop the entire namespace thing then. The fetching without creating a remote is good enough by itself.

wilzbach · 2017-02-26T10:32:40Z

What the heck. I can't get it to work either.

FWIW I think that for the few people that use this script (currently only @MartinNowak), it won't matter anyways. Imho the most important thing is that it's displated on DAutoTest ;-)

wilzbach · 2017-02-26T10:34:19Z

OK, I say to just drop the entire namespace thing then. The fetching without creating a remote is good enough by itself.

Okay - done :)

MartinNowak · 2017-02-27T13:19:36Z

LGTM

Handle empty issue list more gently

44071e0

wilzbach mentioned this pull request Feb 18, 2017

Add pending_changelog target dlang/dlang.org#1549

Merged

CyberShadow assigned MartinNowak Feb 18, 2017

CyberShadow reviewed Feb 18, 2017

View reviewed changes

wilzbach force-pushed the fix-empty-bugzilla branch from f17490e to e049894 Compare February 26, 2017 01:51

CyberShadow requested changes Feb 26, 2017

View reviewed changes

wilzbach changed the title ~~Handle empty issue list more gently~~ Handle empty issue list more gently + avoid fetching the git remote directly Feb 26, 2017

Avoid fetching the remote directly

97253e9

Use a git NAMESPACE to avoid interference with the user's refs.

wilzbach force-pushed the fix-empty-bugzilla branch from e049894 to 97253e9 Compare February 26, 2017 10:00

CyberShadow approved these changes Feb 26, 2017

View reviewed changes

Remove GIT_NAMESPACE

2e0f669

CyberShadow merged commit f2128fb into dlang:master Feb 26, 2017

wilzbach deleted the fix-empty-bugzilla branch February 26, 2017 16:54

Uh oh!

Conversation

wilzbach commented Feb 17, 2017

Uh oh!

CyberShadow commented Feb 18, 2017

Uh oh!

CyberShadow Feb 18, 2017

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Feb 18, 2017

Uh oh!

CyberShadow commented Feb 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Feb 26, 2017

Uh oh!

CyberShadow commented Feb 26, 2017

Uh oh!

CyberShadow Feb 26, 2017

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Feb 26, 2017

Uh oh!

wilzbach commented Feb 26, 2017

Uh oh!

CyberShadow commented Feb 26, 2017

Uh oh!

CyberShadow commented Feb 26, 2017

Uh oh!

CyberShadow commented Feb 26, 2017

Uh oh!

wilzbach commented Feb 26, 2017

Uh oh!

wilzbach commented Feb 26, 2017

Uh oh!

MartinNowak commented Feb 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CyberShadow commented Feb 18, 2017 •

edited

Loading