FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

`options` not reusable, nodegit destroys it · Issue #533 · nodegit/nodegit · GitHub

Repository navigation

options not reusable, nodegit destroys it #533

Description

https://github.com/nodegit/nodegit/blob/master/lib/clone.js#L21

Why are you doing this? This prevents reuse, e.g. I want to clone, and then fetch.

let remoteOpts = {
    remoteCallbacks: {
        certificateCheck: function () {
            return 1;
        },
        credentials: function (url, username) {
            return git.Cred.sshKeyFromAgent(username);
        }
    }
};

function getFileFromBranch(branch, filePath) {
    let repository;

    return git.Repository.open(config.git.cloneTo)
        .catch(function (err) {
            if (err.message.startsWith('Failed to resolve')) {
                winston.info('Repo not found, cloning');

                // USED HERE
                return git.Clone(config.git.repo, config.git.cloneTo, remoteOpts);
            } else {
                winston.error(err);
                throw err;
            }
        })
        .then(function (repo) {
            repository = repo;

            winston.log('verbose', 'Changing branch');
            return repo.checkoutBranch(branch);
        })
        .catch(function (err) {
            if (/Reference '([^']+)' not found/.test(err.message)) {
                winston.info('Branch not found, fetching');

                // USED HERE
                let remoteCallbacks = remoteOpts.remoteCallbacks;
                return repository.fetch('origin', remoteCallbacks, true)
                    .then(function () {
                        return repository.checkoutBranch(branch);
                    });
            } else {
                winston.error(err);
                throw err;
            }
        })
        .then(function () {
            return fs.readFile(path.join(config.git.cloneTo, filePath));
        });
}

Activity

  1. tbranyen commented on Apr 9, 2015

    Member

    As you can see only a few lines further down, we reattach it with the options normalized:

    https://github.com/nodegit/nodegit/blob/master/lib/clone.js#L27-L28

    Your code should work just fine. If not please let us know what's broken.

  2. callumacrae commented on Apr 9, 2015

    Author

    My code was throwing an error because remoteOpts.remoteCallbacks was undefined. Not sure why, but it isn't my code!

  3. tbranyen commented on Apr 9, 2015

    Member

    @callumacrae ah I see what the issue is. We normalize the entire options object thus overriding the one you passed. I'm not entirely sure why we have the delete there anymore... I'll try removing it and see if the tests still pass.

  4. tbranyen commented on Apr 9, 2015

    Member

    Created a PR: #534

  5. tbranyen commented on Apr 9, 2015

    Member

    Yup, here is the bug that is introduced by removing the delete keyword:

    https://travis-ci.org/nodegit/nodegit/jobs/57813650#L716

    We might be able to work around it tho.

  6. johnhaley81 commented on Apr 9, 2015

    Collaborator

    What would we do without tests? We'd break code that's what we'd do!

  7. johnhaley81 commented on Apr 20, 2015

    Collaborator

    @tbranyen did you have an idea for a work-around?

  8. self-assigned this
    on Apr 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


    Back | FazBrowse Home | New Git URL