Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Add ability to override function inside import packages. #21832

Closed
ghost opened this issue Sep 11, 2017 · 8 comments
Closed

Proposal: Add ability to override function inside import packages. #21832

ghost opened this issue Sep 11, 2017 · 8 comments

Comments

@ghost
Copy link

ghost commented Sep 11, 2017

In golang, very common usage of package was import package. packages provided rich library to resolve problem, such as a dns package, a socks packages.

Sometimes we want to working base on a package, but want to override one function to replace the origin package provide us. But we do not want fork a complete new package repository.

For a real word example. I am working on a go http proxy server.

https://github.com/netroby/gohttpproxy

Which use google/martian package as upstream libraries. https://github.com/google/martian

But google/martian package do not support socks5 upstream proxy dialer. So i have to fork the whole google/martian , to modify it's code (just one line of a function)

It's hard for us to do that stupid works.

If golang can let us override the import packages function , our code reuse will be very easier than now.

such as following code

package main

import (
	"crypto/tls"
	"flag"
	"fmt"
	"log"
	"net"
	"net/http"
	"net/url"
	"os"
	"os/signal"
	"time"

	"syscall"

	"github.com/netroby/martian"
	"golang.org/x/net/proxy"
)

var (
	addr    = flag.String("addr", ":8080", "host:port of the proxy")
	forward = flag.String("forward", "", "forward to upstream proxy, example: socks5://127.0.0.1:1080")
)
func (p *Proxy) connect(req *http.Request) (*http.Response, net.Conn, error)  for martian {
	if p.proxyURL != nil {
		log.Debugf("martian: CONNECT with downstream proxy: %s", p.proxyURL.Host)

		conn, err := net.Dial("tcp", p.proxyURL.Host)
		if err != nil {
			return nil, nil, err
		}
		pbw := bufio.NewWriter(conn)
		pbr := bufio.NewReader(conn)

		req.Write(pbw)
		pbw.Flush()

		res, err := http.ReadResponse(pbr, req)
		if err != nil {
			return nil, nil, err
		}

		return res, conn, nil
	}

	log.Debugf("martian: CONNECT to host directly: %s", req.URL.Host)

	conn, err := p.roundTripper.(*http.Transport).Dial("tcp", req.URL.Host)
	if err != nil {
		return nil, nil, err
	}

	return proxyutil.NewResponse(200, nil, req), conn, nil
}
func main() {
//TODO: 
}

func ****() () for martian means the func will replace martian proxy.connect

if we have this , we do not need fork the whole repository and modify code . we can maintain our code . working like a boss.

@davecheney davecheney changed the title [RFC] Add ability to override function inside import packages. Proposal: Add ability to override function inside import packages. Sep 11, 2017
@gopherbot gopherbot added this to the Proposal milestone Sep 11, 2017
@davecheney
Copy link
Contributor

Thank you for your proposal.

Can you please explain why you need to change the language to achieve this. Is there no other way to do this without changing the language? You mention that you have already forked the google/martian repository. Why is this not sufficient? For example, have you considered contributing your improvement upstream?

@ccbrown
Copy link

ccbrown commented Sep 11, 2017

This just looks like dependency hell to me. If you override a function from package foo, every package that depends on foo (including foo itself) also depends on you now. Instant cyclic dependency. I just don't see how this is practical from either a compiler standpoint or an organizational standpoint.

@ghost
Copy link
Author

ghost commented Sep 11, 2017

@ccbrown As a app, we can safe override upstream package function. It will benefit not to fork upstream and maintain the huge repository.

@davecheney I have no idea how to deal this with golang, in other Programming languge, like java, php, or python. CSharp
We can easy extends class. and override one function of origin class.

See csharp polymorphism
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/polymorphism

public class Shape
{
    // A few example members
    public int X { get; private set; }
    public int Y { get; private set; }
    public int Height { get; set; }
    public int Width { get; set; }
   
    // Virtual method
    public virtual void Draw()
    {
        Console.WriteLine("Performing base class drawing tasks");
    }
}

class Circle : Shape
{
    public override void Draw()
    {
        // Code to draw a circle...
        Console.WriteLine("Drawing a circle");
        base.Draw();
    }
}
class Rectangle : Shape
{
    public override void Draw()
    {
        // Code to draw a rectangle...
        Console.WriteLine("Drawing a rectangle");
        base.Draw();
    }
}
class Triangle : Shape
{
    public override void Draw()
    {
        // Code to draw a triangle...
        Console.WriteLine("Drawing a triangle");
        base.Draw();
    }
}

class Program
{
    static void Main(string[] args)
    {
        // Polymorphism at work #1: a Rectangle, Triangle and Circle
        // can all be used whereever a Shape is expected. No cast is
        // required because an implicit conversion exists from a derived 
        // class to its base class.
        System.Collections.Generic.List<Shape> shapes = new System.Collections.Generic.List<Shape>();
        shapes.Add(new Rectangle());
        shapes.Add(new Triangle());
        shapes.Add(new Circle());

        // Polymorphism at work #2: the virtual method Draw is
        // invoked on each of the derived classes, not the base class.
        foreach (Shape s in shapes)
        {
            s.Draw();
        }

        // Keep the console open in debug mode.
        Console.WriteLine("Press any key to exit.");
        Console.ReadKey();
    }

}

@cznic
Copy link
Contributor

cznic commented Sep 11, 2017

We can easy extends class. and override one function of origin class.

Go has no classes and non-interface types have no inheritance concept.

@ghost
Copy link
Author

ghost commented Sep 11, 2017

@cznic, as the example. package github.com/google/martian has

func (p *Proxy) connect(req *http.Request) (*http.Response, net.Conn, error)  

function connect belong to struct Proxy

I need to replace this function to support socks5 proxy dialer

@ghost
Copy link
Author

ghost commented Sep 11, 2017

https://github.com/google/martian/pull/214/files
The PR i submit to google/martian. not yet processed. seems they will not accept .
So i have to fork the repository (It should not do like this kind hard work)

@maj-o
Copy link

maj-o commented Sep 18, 2017

@netroby: If they do not accept your submit, maybe they do accept this: If I know that there may be a different implementation of something I declare a function type. And any instance
of a class has a property of this type (usualy nil). But if this property is set, this function will be called not the default function. This is accepable. I think (hope).

@rsc
Copy link
Contributor

rsc commented Sep 18, 2017

In general, part of authoring a package means you have control over what you allow clients to override and what you do not. If you haven't designed your package to allow an override, clients should not be able to override it. Otherwise you end up with clients that depend on internal details and then you can't ever change those internal details, even though you tried to keep them internal. We understand that this is different from languages like Python where monkey-patching is common, but it's a key part of scalable software development to have hard boundaries like this.

If a package you want to use doesn't enable the overrides you need, the only two options are either fork that package or ask the upstream maintainers to add the kind of override you want. But we aren't going to add support in the language or toolchain for arbitrary overrides, because it makes it too difficult to scale to large software projects.

@rsc rsc closed this as completed Sep 18, 2017
@golang golang locked and limited conversation to collaborators Sep 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants