Warn for existence of _checkouts

[paulo-ferraz-oliveira]

Closes #68.

~I'm not sure a warning is enough, but also think rebar3 can't stop on :init (at least, I know not how to), so I chose this.~

~I'm proposing we simply ignore it, as suggested by @starbelly below.~

Re-reading the linked issue, this doesn't seem right. I'm again going for the warning approach.

[paulo-ferraz-oliveira]

This errors out with

✗ rebar3 hex publish
===> Analyzing applications...
===> Compiling rebar3_hex
===> Verifying dependencies...
===> _checkouts directory found; this might interfere with publishing

We can always add an option to not error out and assume the publisher knows what it's doing.

[starbelly]

This errors out with

✗ rebar3 hex publish
===> Analyzing applications...
===> Compiling rebar3_hex
===> Verifying dependencies...
===> _checkouts directory found; this might interfere with publishing

We can always add an option to not error out and assume the publisher knows what it's doing.

So I think I'd prefer a warning myself. Let me give you a case 🧐 ... the case of publishing rebar3_hex itself. We intentionally put a _checkouts dir in rebar3_hex when publishing a new version as a form of QA if you will. Sometimes because we have to, can't remember off hand that "have to" condition, but yeah still a valid case. Thus it's best if we just ignore the dir and warn about it IMO. Ignoring it is simple ofc.

[paulo-ferraz-oliveira]

So I think I'd prefer a warning myself. Let me give you a case 🧐 ... the case of publishing rebar3_hex itself. We intentionally put a _checkouts dir in rebar3_hex when publishing a new version as a form of QA if you will. Sometimes because we have to, can't remember off hand that "have to" condition, but yeah still a valid case. Thus it's best if we just ignore the dir and warn about it IMO. Ignoring it is simple ofc.

I believe it's ignored already, isn't it? Otherwise, how are you doing it in your use case?

As for the warning, sure, but @tsloughter just pointed out, earlier, that it might get hidden if a release is huge and involves tons of files.

Edit: moved back to a non-blocking/stopping warning.

[starbelly]

As for the warning, sure, but @tsloughter just pointed out, earlier, that it might get hidden if a release is huge and involves tons of files.

Yeah, that makes sense, which is why we should filter out _checkouts dir when we expand paths and build up dirs and files for the tarball that gets created.

Edit:

Forgot to note, right now we don't explicitly ignore _checkouts, rather we just don't put it in the default includes (see line 22 in rebar3_hex_publish).

But Tristan raises a case that hasn't been addressed, but could be by explicitly excluding any _checkouts dir. So take the case of someone having a _checkouts dir in src (i.e., src/_checkouts (contrived example), that would end up in the tarball. We don't want that, and if someone really would want it, we should make it so they can explicitly opt in to include it.

So, with how you're warning is right now, it's just case the simplest case possible, which is fine for a start and this PR IMO.

Now, how does _checkouts work when we publish rebar3_hex?

1. cd into rebar3_hex dir
2. mkdir _checkouts
3. copy the contents of rebar3_hex into _checkout/rebar3_hex or do a new git clone
4. rebar3 hex publish

rebar3 will load rebar3_hex vs using the plugin that's in deps or global plugins. I probably was a bit confusing when I talked about that, so figured a better explanation of that case was due.

[paulo-ferraz-oliveira]

Yeah, that makes sense, which is why we should filter out _checkouts dir when we expand paths and build up dirs and files for the tarball that gets created.

In that sense, a warning is not required, right? Unless it is to tell you what you're publishing might not work as you expect it to.

[paulo-ferraz-oliveira]

we should make it so they can explicitly opt in to include it.

Does an option for this already exist?

[paulo-ferraz-oliveira]

which is fine for a start and this PR IMO.

I don't mind adding more, but I need explicit orientation. You want a warning + excluding _checkouts?

[starbelly]

I don't mind adding more, but I need explicit orientation. You want a warning + excluding _checkouts?

Yup, warn + exclude, which resolves #68

[paulo-ferraz-oliveira]

Hm... if I just add _checkouts$ to the filter it'll probably filter more than <cwd>/_checkouts. Any proper way you want that or are you Ok with just _checkouts$?

[starbelly]

Hm... if I just add _checkouts$ to the filter it'll probably filter more than <cwd>/_checkouts. Any proper way you want that or are you Ok with just _checkouts$?

Yeah, that would be the idea, catch all the instances of _checkouts. Further, warn about each instance found. I think a warning is prudent as an author might actually have a valid case where they do not want to exclude _checkouts dir. Giving them a warning would at least give them the chance to back out of the operation and modify their config to include _checkouts and we will have had made a best effort at sending them down the "right" path.

[paulo-ferraz-oliveira]

Sure, but the warning would be printed potentially before the file/folder listing so might get lost, at least visually, right?

[paulo-ferraz-oliveira]

So this is what it's doing now:

1. it warns about the existence of _checkouts (per app, even in umbrella),
2. it changes the "Proceed?" message to "Proceed (with warnings)?" where warranted.